swcs/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

U4-9134 XSS security issue in the grid

Browse Source 
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.
This commit is contained in:
Claus
2016-11-08 09:55:24 +01:00
parent a0c672eb91
commit 8bb069e996
12 changed files with 164 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Umbraco.Core/StringExtensions.cs
+1 -1
View File
@@ -184,7 +184,7 @@ namespace Umbraco.Core
/// <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();
src/Umbraco.Web.UI/Umbraco.Web.UI.csproj
+2
View File
@@ -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>
src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml
+21 -10
View File
@@ -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));
}
}
src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml
+21 -10
View File
@@ -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));
}
}
src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml
+22 -10
View File
@@ -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));
}
}
src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml
+21 -10
View File
@@ -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));
}
}
src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Base.cshtml
-1
View File
@@ -1,5 +1,4 @@
@model dynamic
@using Umbraco.Web.Templates
@functions {
public static string EditorView(dynamic contentItem)
src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Embed.cshtml
-1
View File
@@ -1,3 +1,2 @@
@model dynamic
@using Umbraco.Web.Templates
@Html.Raw(Model.value)
src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Macro.cshtml
-2
View File
@@ -1,6 +1,4 @@
@inherits UmbracoViewPage<dynamic>
@using Umbraco.Web.Templates
@if (Model.value != null)
{
src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Media.cshtml
-1
View File
@@ -1,5 +1,4 @@
@model dynamic
@using Umbraco.Web.Templates
@if (Model.value != null)
{
src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml
+2 -3
View File
@@ -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>
}
src/Umbraco.Web/Templates/TemplateUtilities.cs
+74 -69
View File
@@ -7,17 +7,17 @@ using Umbraco.Core.Logging;
namespace Umbraco.Web.Templates
{
//NOTE: I realize there is only one class in this namespace but I'm pretty positive that there will be more classes in
//this namespace once we start migrating and cleaning up more code.
//NOTE: I realize there is only one class in this namespace but I'm pretty positive that there will be more classes in
//this namespace once we start migrating and cleaning up more code.
/// <summary>
/// Utility class used for templates
/// </summary>
public static class TemplateUtilities
{
/// <summary>
/// Utility class used for templates
/// </summary>
public static class TemplateUtilities
{
//TODO: Pass in an Umbraco context!!!!!!!! Don't rely on the singleton so things are more testable
internal static string ParseInternalLinks(string text, bool preview)
{
{
// save and set for url provider
var inPreviewMode = UmbracoContext.Current.InPreviewMode;
UmbracoContext.Current.InPreviewMode = preview;
@@ -33,79 +33,84 @@ namespace Umbraco.Web.Templates
}
return text;
}
}
/// <summary>
/// Parses the string looking for the {localLink} syntax and updates them to their correct links.
/// </summary>
/// <param name="text"></param>
/// <returns></returns>
public static string ParseInternalLinks(string text)
{
/// <summary>
/// Parses the string looking for the {localLink} syntax and updates them to their correct links.
/// </summary>
/// <param name="text"></param>
/// <returns></returns>
public static string ParseInternalLinks(string text)
{
//TODO: Pass in an Umbraco context!!!!!!!! Don't rely on the singleton so things are more testable, better yet, pass in urlprovider, routing context, separately
//don't attempt to proceed without a context as we cannot lookup urls without one
if (UmbracoContext.Current == null || UmbracoContext.Current.RoutingContext == null)
{
return text;
}
//don't attempt to proceed without a context as we cannot lookup urls without one
if (UmbracoContext.Current == null || UmbracoContext.Current.RoutingContext == null)
{
return text;
}
var urlProvider = UmbracoContext.Current.UrlProvider;
var urlProvider = UmbracoContext.Current.UrlProvider;
// Parse internal links
var tags = Regex.Matches(text, @"href=""[/]?(?:\{|\%7B)localLink:([0-9]+)(?:\}|\%7D)", RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace);
foreach (Match tag in tags)
if (tag.Groups.Count > 0)
{
var id = tag.Groups[1].Value; //.Remove(tag.Groups[1].Value.Length - 1, 1);
var newLink = urlProvider.GetUrl(int.Parse(id));
text = text.Replace(tag.Value, "href=\"" + newLink);
}
// Parse internal links
var tags = Regex.Matches(text, @"href=""[/]?(?:\{|\%7B)localLink:([0-9]+)(?:\}|\%7D)", RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace);
foreach (Match tag in tags)
if (tag.Groups.Count > 0)
{
var id = tag.Groups[1].Value; //.Remove(tag.Groups[1].Value.Length - 1, 1);
var newLink = urlProvider.GetUrl(int.Parse(id));
text = text.Replace(tag.Value, "href=\"" + newLink);
}
return text;
}
}
// static compiled regex for faster performance
private readonly static Regex ResolveUrlPattern = new Regex("(=[\"\']?)(\\W?\\~(?:.(?![\"\']?\\s+(?:\\S+)=|[>\"\']))+.)[\"\']?", RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace);
// static compiled regex for faster performance
private readonly static Regex ResolveUrlPattern = new Regex("(=[\"\']?)(\\W?\\~(?:.(?![\"\']?\\s+(?:\\S+)=|[>\"\']))+.)[\"\']?",
RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace);
/// <summary>
/// The RegEx matches any HTML attribute values that start with a tilde (~), those that match are passed to ResolveUrl to replace the tilde with the application path.
/// </summary>
/// <param name="text"></param>
/// <returns></returns>
/// <remarks>
/// When used with a Virtual-Directory set-up, this would resolve all URLs correctly.
/// The recommendation is that the "ResolveUrlsFromTextString" option (in umbracoSettings.config) is set to false for non-Virtual-Directory installs.
/// </remarks>
public static string ResolveUrlsFromTextString(string text)
{
/// <summary>
/// The RegEx matches any HTML attribute values that start with a tilde (~), those that match are passed to ResolveUrl to replace the tilde with the application path.
/// </summary>
/// <param name="text"></param>
/// <returns></returns>
/// <remarks>
/// When used with a Virtual-Directory set-up, this would resolve all URLs correctly.
/// The recommendation is that the "ResolveUrlsFromTextString" option (in umbracoSettings.config) is set to false for non-Virtual-Directory installs.
/// </remarks>
public static string ResolveUrlsFromTextString(string text)
{
if (UmbracoConfig.For.UmbracoSettings().Content.ResolveUrlsFromTextString == false) return text;
using (var timer = DisposableTimer.DebugDuration(typeof(IOHelper), "ResolveUrlsFromTextString starting", "ResolveUrlsFromTextString complete"))
{
// find all relative urls (ie. urls that contain ~)
var tags = ResolveUrlPattern.Matches(text);
LogHelper.Debug(typeof(IOHelper), "After regex: " + timer.Stopwatch.ElapsedMilliseconds + " matched: " + tags.Count);
foreach (Match tag in tags)
{
var url = "";
if (tag.Groups[1].Success)
url = tag.Groups[1].Value;
using (var timer = DisposableTimer.DebugDuration(typeof(IOHelper), "ResolveUrlsFromTextString starting", "ResolveUrlsFromTextString complete"))
{
// find all relative urls (ie. urls that contain ~)
var tags = ResolveUrlPattern.Matches(text);
LogHelper.Debug(typeof(IOHelper), "After regex: " + timer.Stopwatch.ElapsedMilliseconds + " matched: " + tags.Count);
foreach (Match tag in tags)
{
var url = "";
if (tag.Groups[1].Success)
url = tag.Groups[1].Value;
// The richtext editor inserts a slash in front of the url. That's why we need this little fix
// if (url.StartsWith("/"))
// text = text.Replace(url, ResolveUrl(url.Substring(1)));
// else
if (String.IsNullOrEmpty(url) == false)
{
var resolvedUrl = (url.Substring(0, 1) == "/") ? IOHelper.ResolveUrl(url.Substring(1)) : IOHelper.ResolveUrl(url);
text = text.Replace(url, resolvedUrl);
}
}
}
// The richtext editor inserts a slash in front of the url. That's why we need this little fix
// if (url.StartsWith("/"))
// text = text.Replace(url, ResolveUrl(url.Substring(1)));
// else
if (String.IsNullOrEmpty(url) == false)
{
var resolvedUrl = (url.Substring(0, 1) == "/") ? IOHelper.ResolveUrl(url.Substring(1)) : IOHelper.ResolveUrl(url);
text = text.Replace(url, resolvedUrl);
}
}
}
return text;
}
return text;
}
}
public static string CleanForXss(string text, params char[] ignoreFromClean)
{
return text.CleanForXss(ignoreFromClean);
}
}
}