From f2fb200f1bb10b34b80bda60753cdaa11264fa0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caner=20Pat=C4=B1r?= Date: Sun, 6 May 2018 19:54:29 +0300 Subject: [PATCH] more test --- src/AntiSamy/AntiSamyDomScanner.cs | 210 +++++++-------- src/AntiSamy/CssScanner.cs | 23 +- test/AntiSamy.Tests/AntiSamy.Tests.csproj | 4 + test/AntiSamy.Tests/AntiSamyTests.cs | 240 ++++++++++++++++-- .../resources/antisamy-ebay.xml | 14 - .../resources/antisamy-myspace.xml | 14 - 6 files changed, 355 insertions(+), 150 deletions(-) diff --git a/src/AntiSamy/AntiSamyDomScanner.cs b/src/AntiSamy/AntiSamyDomScanner.cs index 22d1339..8e63df2 100644 --- a/src/AntiSamy/AntiSamyDomScanner.cs +++ b/src/AntiSamy/AntiSamyDomScanner.cs @@ -147,104 +147,19 @@ namespace AntiSamy { if ("style".Equals(tagName.ToLower()) && _policy.GetTag("style") != null) { - ScanCss(node, parentNode, maxinputsize); + ScanCss(node, parentNode, maxinputsize, false); } for (var currentAttributeIndex = 0; currentAttributeIndex < node.Attributes.Count; currentAttributeIndex++) { - HtmlAttribute attribute = node.Attributes[currentAttributeIndex]; + HtmlAttribute htmlAttribute = node.Attributes[currentAttributeIndex]; - string name = attribute.Name; - string value = attribute.Value; + string name = htmlAttribute.Name; + string value = htmlAttribute.Value; - DocumentAttribute attr = tag.GetAttributeByName(name) ?? _policy.GetGlobalAttribute(name); - var isAttributeValid = false; - - if ("style".Equals(name.ToLower()) && attr != null) - { - ScanCss(node, parentNode, maxinputsize); - } - if (attr != null) - { - //try to find out how robust this is - do I need to do this in a loop? - value = HtmlEntity.DeEntitize(value); - - foreach (string allowedValue in attr.AllowedValues) - { - if (isAttributeValid) - { - break; - } - - if (allowedValue != null && allowedValue.ToLower().Equals(value.ToLower())) - { - isAttributeValid = true; - } - } - - foreach (string ptn in attr.AllowedRegExps) - { - if (isAttributeValid) - { - break; - } - string pattern = "^" + ptn + "$"; - Match m = Regex.Match(value, pattern); - if (m.Success) - { - isAttributeValid = true; - } - } - - if (!isAttributeValid) - { - string onInvalidAction = attr.OnInvalid; - var errBuff = new StringBuilder(); - - errBuff.Append("The " + HtmlEntityEncoder.HtmlEntityEncode(tagName) + " tag contained an attribute that we couldn't process. "); - errBuff.Append("The " + HtmlEntityEncoder.HtmlEntityEncode(name) + " attribute had a value of " + HtmlEntityEncoder.HtmlEntityEncode(value) + ". "); - errBuff.Append("This value could not be accepted for security reasons. We have chosen to "); - - //Console.WriteLine(policy); - - if (Consts.OnInvalidActions.REMOVE_TAG.Equals(onInvalidAction)) - { - parentNode.RemoveChild(node); - errBuff.Append("remove the " + HtmlEntityEncoder.HtmlEntityEncode(tagName) + " tag and its contents in order to process this input. "); - } - else if (Consts.OnInvalidActions.FILTER_TAG.Equals(onInvalidAction)) - { - for (var i = 0; i < node.ChildNodes.Count; i++) - { - tmp = node.ChildNodes[i]; - RecursiveValidateTag(tmp); - if (tmp.ParentNode == null) - { - i--; - } - } - - PromoteChildren(node); - - errBuff.Append("filter the " + HtmlEntityEncoder.HtmlEntityEncode(tagName) + " tag and leave its contents in place so that we could process this input."); - } - else if (Consts.OnInvalidActions.REMOVE_ATTRIBUTE.Equals(onInvalidAction)) - { - node.Attributes.Remove(attr.Name); - currentAttributeIndex--; - errBuff.Append("remove the " + HtmlEntityEncoder.HtmlEntityEncode(name) + " attribute from the tag and leave everything else in place so that we could process this input."); - } - - _errorMessages.Add(errBuff.ToString()); - - if ("removeTag".Equals(onInvalidAction) || "filterTag".Equals(onInvalidAction)) - { - return; // can't process any more if we remove/filter the tag - } - } - } - else + DocumentAttribute allowwdAttr = tag.GetAttributeByName(name) ?? _policy.GetGlobalAttribute(name); + if (allowwdAttr == null) { var errBuff = new StringBuilder(); @@ -256,6 +171,94 @@ namespace AntiSamy node.Attributes.Remove(name); currentAttributeIndex--; } + else + { + + if ("style".Equals(name.ToLower()) && allowwdAttr != null) + { + ScanCss(node, parentNode, maxinputsize, true); + } + else + { + var isAttributeValid = false; + //try to find out how robust this is - do I need to do this in a loop? + value = HtmlEntity.DeEntitize(value); + + foreach (string allowedValue in allowwdAttr.AllowedValues) + { + if (isAttributeValid) + { + break; + } + + if (allowedValue != null && allowedValue.ToLower().Equals(value.ToLower())) + { + isAttributeValid = true; + } + } + + foreach (string ptn in allowwdAttr.AllowedRegExps) + { + if (isAttributeValid) + { + break; + } + string pattern = "^" + ptn + "$"; + Match m = Regex.Match(value, pattern); + if (m.Success) + { + isAttributeValid = true; + } + } + + if (!isAttributeValid) + { + string onInvalidAction = allowwdAttr.OnInvalid; + var errBuff = new StringBuilder(); + + errBuff.Append("The " + HtmlEntityEncoder.HtmlEntityEncode(tagName) + " tag contained an attribute that we couldn't process. "); + errBuff.Append("The " + HtmlEntityEncoder.HtmlEntityEncode(name) + " attribute had a value of " + HtmlEntityEncoder.HtmlEntityEncode(value) + ". "); + errBuff.Append("This value could not be accepted for security reasons. We have chosen to "); + + //Console.WriteLine(policy); + + if (Consts.OnInvalidActions.REMOVE_TAG.Equals(onInvalidAction)) + { + parentNode.RemoveChild(node); + errBuff.Append("remove the " + HtmlEntityEncoder.HtmlEntityEncode(tagName) + " tag and its contents in order to process this input. "); + } + else if (Consts.OnInvalidActions.FILTER_TAG.Equals(onInvalidAction)) + { + for (var i = 0; i < node.ChildNodes.Count; i++) + { + tmp = node.ChildNodes[i]; + RecursiveValidateTag(tmp); + if (tmp.ParentNode == null) + { + i--; + } + } + + PromoteChildren(node); + + errBuff.Append("filter the " + HtmlEntityEncoder.HtmlEntityEncode(tagName) + " tag and leave its contents in place so that we could process this input."); + } + else + { + node.Attributes.Remove(allowwdAttr.Name); + currentAttributeIndex--; + errBuff.Append("remove the " + HtmlEntityEncoder.HtmlEntityEncode(name) + " attribute from the tag and leave everything else in place so that we could process this input."); + } + + _errorMessages.Add(errBuff.ToString()); + + if ("removeTag".Equals(onInvalidAction) || "filterTag".Equals(onInvalidAction)) + { + return; // can't process any more if we remove/filter the tag + } + } + } + } } for (var i = 0; i < node.ChildNodes.Count; i++) @@ -268,7 +271,7 @@ namespace AntiSamy } } } - else if ("truncate".Equals(tag.Action) || Consts.TagActions.REMOVE.Equals(tag.Action)) + else if ("truncate".Equals(tag.Action))// || Consts.TagActions.REMOVE.Equals(tag.Action)) { Console.WriteLine("truncate"); HtmlAttributeCollection nnmap = node.Attributes; @@ -311,23 +314,30 @@ namespace AntiSamy } } - private void ScanCss(HtmlNode node, HtmlNode parentNode, int maxinputsize) + private void ScanCss(HtmlNode node, HtmlNode parentNode, int maxinputsize, bool fromStyleAttribute) { var styleScanner = new CssScanner(_policy); try { - AntiySamyResult cssResult; - if (node.Attributes.Contains("style")) + AntiySamyResult cssResult = null; + //if (node.Attributes.Contains("style")) + if (fromStyleAttribute) { - cssResult = styleScanner.ScanStyleSheet(node.Attributes["style"].Value, maxinputsize); - node.Attributes["style"].Value = cssResult.CleanHtml; + cssResult = styleScanner.ScanStyleSheet(node.Attributes["style"].Value, maxinputsize, fromStyleAttribute); + if (string.IsNullOrWhiteSpace(cssResult.CleanHtml)) + { + node.Attributes["style"].Remove(); + } + else + node.Attributes["style"].Value = cssResult.CleanHtml; } - else + else if (node.FirstChild != null) { - cssResult = styleScanner.ScanStyleSheet(node.FirstChild.InnerHtml, maxinputsize); + cssResult = styleScanner.ScanStyleSheet(node.FirstChild.InnerHtml, maxinputsize, fromStyleAttribute); node.FirstChild.InnerHtml = cssResult.CleanHtml; } - _errorMessages.AddRange(cssResult.ErrorMessages); + if (cssResult != null) + _errorMessages.AddRange(cssResult.ErrorMessages); } catch (ParseException e) { diff --git a/src/AntiSamy/CssScanner.cs b/src/AntiSamy/CssScanner.cs index b596f00..999b481 100644 --- a/src/AntiSamy/CssScanner.cs +++ b/src/AntiSamy/CssScanner.cs @@ -15,12 +15,14 @@ namespace AntiSamy { internal class CssScanner { + private const string dummySelectorBegin = ".dummySelector {"; + private const string dummySelectorEnd = " }"; private readonly Policy _policy; private List _errors = new List(); public CssScanner(Policy policy) => _policy = policy ?? throw new ArgumentNullException(nameof(policy)); - public AntiySamyResult ScanStyleSheet(string css, int maxinputsize) + public AntiySamyResult ScanStyleSheet(string css, int maxinputsize, bool fromStyleAttribute) { DateTime start = DateTime.UtcNow; _errors = new List(); @@ -37,14 +39,15 @@ namespace AntiSamy IsIncludingUnknownRules = true, IsToleratingInvalidConstraints = true, IsToleratingInvalidValues = true - }).ParseStylesheet(css); + }).ParseStylesheet(fromStyleAttribute ? dummySelectorBegin + css + dummySelectorEnd : css); } catch (Exception ex) { throw new ParseException(ex.Message, ex); } - cleanStyleSheet = ScanStyleSheet(styleSheet); + var result = ScanStyleSheet(styleSheet); + cleanStyleSheet = fromStyleAttribute ? CleanDummyWrapper(result) : result; } catch (ParseException) { @@ -58,6 +61,20 @@ namespace AntiSamy return new AntiySamyResult(start, cleanStyleSheet, _errors); } + private string CleanDummyWrapper(string result) + { + if (result.StartsWith(dummySelectorBegin)) + { + result = result.Replace(dummySelectorBegin, string.Empty); + + if (result.EndsWith("}")) + { + result = result.Remove(result.Length - 1); + } + } + return string.IsNullOrWhiteSpace(result) ? string.Empty : result; + } + private string ScanStyleSheet(ICssStyleSheet styleSheet) { for (var i = 0; i < styleSheet.Rules.Length;) diff --git a/test/AntiSamy.Tests/AntiSamy.Tests.csproj b/test/AntiSamy.Tests/AntiSamy.Tests.csproj index 6dfe533..6661db5 100644 --- a/test/AntiSamy.Tests/AntiSamy.Tests.csproj +++ b/test/AntiSamy.Tests/AntiSamy.Tests.csproj @@ -46,4 +46,8 @@ + + + + diff --git a/test/AntiSamy.Tests/AntiSamyTests.cs b/test/AntiSamy.Tests/AntiSamyTests.cs index 7b6583f..86b9349 100644 --- a/test/AntiSamy.Tests/AntiSamyTests.cs +++ b/test/AntiSamy.Tests/AntiSamyTests.cs @@ -2,7 +2,8 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; - +using System.Text; +using System.Text.RegularExpressions; using FluentAssertions; using Xunit; @@ -32,19 +33,17 @@ namespace AntiSamy.Tests }; private AntiSamy _sut = new AntiSamy(); + Policy policy = GetPolicy("antisamy.xml"); - - private Policy GetTestPolicy() + private static Policy GetPolicy(string fileName) { string currentDir = Directory.GetCurrentDirectory(); - return Policy.FromFile(Path.Combine(currentDir, @"resources\antisamy.xml")); + return Policy.FromFile(Path.Combine(currentDir, $@"resources\{fileName}")); } - [Fact(Skip = "a")] + [Fact] public void scriptAttacks() { - Policy policy = GetTestPolicy(); - _sut.Scan("test", policy).CleanHtml.Contains("script").Should().BeFalse(); _sut.Scan("<<<><@import 'x';bar"; + _sut.Scan(s, policy); + + } + + [Fact] + public void issue40() + { + /* issue #40 - handling "; + //Policy revised = policy.cloneWithDirective(Policy.PRESERVE_SPACE, "true"); + + var result = _sut.Scan(s, policy); + result.CleanHtml.Contains("print, projection, screen").Should().BeTrue(); + + } + + private void assertEquals(string actual, string expected) + { + actual.Should().BeEquivalentTo(expected); } } } diff --git a/test/AntiSamy.Tests/resources/antisamy-ebay.xml b/test/AntiSamy.Tests/resources/antisamy-ebay.xml index 9f4ef15..c21a783 100644 --- a/test/AntiSamy.Tests/resources/antisamy-ebay.xml +++ b/test/AntiSamy.Tests/resources/antisamy-ebay.xml @@ -549,8 +549,6 @@ http://www.w3.org/TR/html401/struct/global.html - - @@ -2257,18 +2255,6 @@ http://www.w3.org/TR/html401/struct/global.html - - - - - - - - - - - - diff --git a/test/AntiSamy.Tests/resources/antisamy-myspace.xml b/test/AntiSamy.Tests/resources/antisamy-myspace.xml index e3adbab..c1f66d2 100644 --- a/test/AntiSamy.Tests/resources/antisamy-myspace.xml +++ b/test/AntiSamy.Tests/resources/antisamy-myspace.xml @@ -711,8 +711,6 @@ http://www.w3.org/TR/html401/struct/global.html - - @@ -2414,18 +2412,6 @@ http://www.w3.org/TR/html401/struct/global.html - - - - - - - - - - - -