U4-9134 XSS security issue in the grid

exposing xss clean method on templateutilities. making the clean xss string extensions public instead of internal. ensuring the included grid renderers clean for xss. ensuring the included grid editors using html.raw with value directly, cleans for xss.
umbraco · Nov 8, 2016 · 8bb069e · 8bb069e
1 parent a0c672e
commit 8bb069e
Show file tree

Hide file tree

Showing 12 changed files with 164 additions and 118 deletions.
diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs
@@ -184,7 +184,7 @@ internal static string ReplaceNonAlphanumericChars(this string input, char repla
         /// <param name="input"></param>
         /// <param name="ignoreFromClean"></param>
         /// <returns></returns>
-        internal static string CleanForXss(this string input, params char[] ignoreFromClean)
+        public static string CleanForXss(this string input, params char[] ignoreFromClean)
         {
             //remove any html
             input = input.StripHtml();

diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj
@@ -1973,6 +1973,8 @@
     <Content Include="Views\Partials\Grid\Editors\Textstring.cshtml" />
     <Content Include="Views\Partials\Grid\Bootstrap2.cshtml" />
     <Content Include="Views\Partials\Grid\Editors\Base.cshtml" />
+    <Content Include="Views\Partials\Grid\Bootstrap3-Fluid.cshtml" />
+    <Content Include="Views\Partials\Grid\Bootstrap2-Fluid.cshtml" />
     <None Include="Web.Debug.config.transformed" />
     <None Include="web.Template.Debug.config">
       <DependentUpon>Web.Template.config</DependentUpon>

diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml
@@ -64,21 +64,32 @@
         JObject cfg = contentItem.config;
 
         if(cfg != null)
-            foreach (JProperty property in cfg.Properties()) {
-                attrs.Add(property.Name + "='" + property.Value.ToString() + "'");
+            foreach (JProperty property in cfg.Properties())
+            {
+                var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString());
+                if (string.IsNullOrWhiteSpace(propertyValue) == false)
+                {
+                    attrs.Add(property.Name + "='" + propertyValue + "'");
+                }
             }
-                
+
         JObject style = contentItem.styles;
 
-        if (style != null) { 
-        var cssVals = new List<string>();
-        foreach (JProperty property in style.Properties())
-            cssVals.Add(property.Name + ":" + property.Value.ToString() + ";");
+        if (style != null) {
+            var cssVals = new List<string>();
+            foreach (JProperty property in style.Properties())
+            {
+                var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString());
+                if (string.IsNullOrWhiteSpace(propertyValue) == false)
+                {
+                    cssVals.Add(property.Name + ":" + propertyValue + ";");
+                }
+            }
 
-        if (cssVals.Any())
-            attrs.Add("style='" + string.Join(" ", cssVals) + "'");
+            if (cssVals.Any())
+                attrs.Add("style='" + string.Join(" ", cssVals) + "'");
         }
-            
+
         return new MvcHtmlString(string.Join(" ", attrs));
     }
 }
diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml
@@ -64,21 +64,32 @@
         JObject cfg = contentItem.config;
 
         if(cfg != null)
-            foreach (JProperty property in cfg.Properties()) {
-                attrs.Add(property.Name + "=\"" + property.Value.ToString() + "\"");
+            foreach (JProperty property in cfg.Properties())
+            {
+                var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString());
+                if (string.IsNullOrWhiteSpace(propertyValue) == false)
+                {
+                    attrs.Add(property.Name + "=\"" + propertyValue + "\"");
+                }
             }
-                
+
         JObject style = contentItem.styles;
 
-        if (style != null) { 
-        var cssVals = new List<string>();
-        foreach (JProperty property in style.Properties())
-            cssVals.Add(property.Name + ":" + property.Value.ToString() + ";");
+        if (style != null) {
+            var cssVals = new List<string>();
+            foreach (JProperty property in style.Properties())
+            {
+                var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString());
+                if (string.IsNullOrWhiteSpace(propertyValue) == false)
+                {
+                    cssVals.Add(property.Name + ":" + propertyValue + ";");
+                }
+            }
 
-        if (cssVals.Any())
-            attrs.Add("style=\"" + string.Join(" ", cssVals) + "\"");
+            if (cssVals.Any())
+                attrs.Add("style=\"" + string.Join(" ", cssVals) + "\"");
         }
-            
+
         return new MvcHtmlString(string.Join(" ", attrs));
     }
 }
diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml
@@ -5,6 +5,7 @@
 @* 
     Razor helpers located at the bottom of this file
 *@
+
 @if (Model != null && Model.sections != null)
 {
     var oneColumn = ((System.Collections.ICollection)Model.sections).Count == 1;
@@ -59,21 +60,32 @@
         JObject cfg = contentItem.config;
 
         if(cfg != null)
-            foreach (JProperty property in cfg.Properties()) {
-                attrs.Add(property.Name + "='" + property.Value.ToString() + "'");
+            foreach (JProperty property in cfg.Properties())
+            {
+                var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString());
+                if (string.IsNullOrWhiteSpace(propertyValue) == false)
+                {
+                    attrs.Add(property.Name + "='" + propertyValue + "'");
+                }
             }
-                
+
         JObject style = contentItem.styles;
 
-        if (style != null) { 
-        var cssVals = new List<string>();
-        foreach (JProperty property in style.Properties())
-            cssVals.Add(property.Name + ":" + property.Value.ToString() + ";");
+        if (style != null) {
+            var cssVals = new List<string>();
+            foreach (JProperty property in style.Properties())
+            {
+                var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString());
+                if (string.IsNullOrWhiteSpace(propertyValue) == false)
+                {
+                    cssVals.Add(property.Name + ":" + propertyValue + ";");
+                }
+            }
 
-        if (cssVals.Any())
-            attrs.Add("style='" + string.Join(" ", cssVals) + "'");
+            if (cssVals.Any())
+                attrs.Add("style='" + string.Join(" ", cssVals) + "'");
         }
-            
+
         return new MvcHtmlString(string.Join(" ", attrs));
     }
 }
diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml
@@ -64,21 +64,32 @@
         JObject cfg = contentItem.config;
 
         if(cfg != null)
-            foreach (JProperty property in cfg.Properties()) {
-                attrs.Add(property.Name + "=\"" + property.Value.ToString() + "\"");
+            foreach (JProperty property in cfg.Properties())
+            {
+                var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString());
+                if (string.IsNullOrWhiteSpace(propertyValue) == false)
+                {
+                    attrs.Add(property.Name + "=\"" + propertyValue +"\"");
+                }
             }
-                
+
         JObject style = contentItem.styles;
 
-        if (style != null) { 
-        var cssVals = new List<string>();
-        foreach (JProperty property in style.Properties())
-            cssVals.Add(property.Name + ":" + property.Value.ToString() + ";");
+        if (style != null) {
+            var cssVals = new List<string>();
+            foreach (JProperty property in style.Properties())
+            {
+                var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString());
+                if (string.IsNullOrWhiteSpace(propertyValue) == false)
+                {
+                    cssVals.Add(property.Name + ":" + propertyValue + ";");
+                }
+            }
 
-        if (cssVals.Any())
-            attrs.Add("style=\"" + string.Join(" ", cssVals) + "\"");
+            if (cssVals.Any())
+                attrs.Add("style=\"" + string.Join(" ", cssVals) + "\"");
         }
-            
+
         return new MvcHtmlString(string.Join(" ", attrs));
     }
 }
diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Base.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Base.cshtml
@@ -1,5 +1,4 @@
 @model dynamic
-@using Umbraco.Web.Templates
 
 @functions {
     public static string EditorView(dynamic contentItem)

diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Embed.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Embed.cshtml
@@ -1,3 +1,2 @@
 @model dynamic
-@using Umbraco.Web.Templates
 @Html.Raw(Model.value)
diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Macro.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Macro.cshtml
@@ -1,6 +1,4 @@
 @inherits UmbracoViewPage<dynamic>
-@using Umbraco.Web.Templates
-
 
 @if (Model.value != null)
 {

diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Media.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Media.cshtml
@@ -1,5 +1,4 @@
 @model dynamic
-@using Umbraco.Web.Templates
 
 @if (Model.value != null)
 {   

diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml
@@ -4,10 +4,9 @@
 @if (Model.editor.config.markup != null)
 {
     string markup = Model.editor.config.markup.ToString();
-
     var UmbracoHelper = new UmbracoHelper(UmbracoContext.Current);
 
-    markup = markup.Replace("#value#", UmbracoHelper.ReplaceLineBreaksForHtml(Model.value.ToString()));
+    markup = markup.Replace("#value#", UmbracoHelper.ReplaceLineBreaksForHtml(TemplateUtilities.CleanForXss(Model.value.ToString())));
     markup = markup.Replace("#style#", Model.editor.config.style.ToString());
 
     <text>
@@ -17,6 +16,6 @@
 else
 {
     <text>
-        <div style="@Model.editor.config.style">@Model.value</div>
+        <div style="@Model.editor.config.style">@TemplateUtilities.CleanForXss(Model.value.ToString())</div>
     </text>
 }