search

mganss / HtmlSanitizer

Cleans HTML to avoid XSS attacks
MIT License
1.52k stars 198 forks source link

Sanitizer removing inline child combinator CSS selectors #483

Closed jasonwnz closed 8 months ago

jasonwnz commented 8 months ago

The latest version of HtmlSanitizer v8.0.723 is generating different sanitized output to the previous version v8.0.718 - it is removing inline child combinator CSS selectors. Is this intended?

See sample .NET 6 code below using v8.0.723

using Ganss.Xss;

var input = "<style>span>p { font-size: 2em }</style><span><p>I am safe</p></span>";
var sanitizer = new HtmlSanitizer();
sanitizer.RemovingTag += (sender, args) => args.Cancel = true;
string output = sanitizer.Sanitize(input);

Console.WriteLine(input);
Console.WriteLine(output);

The output for v8.0.723 is:

<style>span>p { font-size: 2em }</style><span><p>I am safe</p></span>
<style>p { font-size: 2em }</style><span><p>I am safe</p></span>

The output for v8.0.718 is:

<style>span>p { font-size: 2em }</style><span><p>I am safe</p></span>
<style>span>p { font-size: 2em }</style><span><p>I am safe</p></span>
mganss commented 8 months ago

Unfortunately, this was introduced through the fix for https://github.com/mganss/HtmlSanitizer/security/advisories/GHSA-43cp-6p3q-2pc4

Fixed in 8.0.744 and 8.1.745-beta.

jasonwnz commented 8 months ago

Thanks for the quick fix, although I'm still not sure it's 100% correct?

The output for v8.0.744 is now

<style>span>p { font-size: 2em }</style><span><p>I am safe</p></span>
<style>span\3e p { font-size: 2em }</style><span><p>I am safe</p></span>

but this isn't valid CSS - if you paste this in an HTML doc (as below) and view it in a browser, the style doesn't have any effect.

<!doctype html>
<style>span\3e p { font-size: 2em }</style><span><p>I am safe</p></span>
</html>
mganss commented 8 months ago

I was overeager in CSS escaping > also in addition to < (which is necessary for https://github.com/mganss/HtmlSanitizer/security/advisories/GHSA-8j9v-h2vp-2hhv). It should now be fixed finally in 8.0.746 and 8.1.747-beta.

To be honest, I don't understand why span\3e p doesn't work. IMO, the escape should be resolved at the CSS tokenization stage and behave like span>p. Other escapes work fine, e.g. s\70 an>p or p { \63 color: red }.