Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sanitizer removing inline child combinator CSS selectors #483

Closed
jasonwnz opened this issue Oct 20, 2023 · 3 comments
Closed

Sanitizer removing inline child combinator CSS selectors #483

jasonwnz opened this issue Oct 20, 2023 · 3 comments

Comments

@jasonwnz
Copy link

jasonwnz commented Oct 20, 2023 •
edited
Loading

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 mganss closed this as completed in fe337df Oct 23, 2023
mganss added a commit that referenced this issue Oct 23, 2023
@mganss
Fix #483
3bb56c0
@mganss
Copy link
Owner

mganss commented Oct 23, 2023

Unfortunately, this was introduced through the fix for GHSA-43cp-6p3q-2pc4

Fixed in 8.0.744 and 8.1.745-beta.

@jasonwnz
Copy link
Author

jasonwnz commented Oct 23, 2023 •
edited
Loading

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 mganss reopened this Oct 24, 2023
@mganss mganss closed this as completed in e3e943f Oct 24, 2023
mganss added a commit that referenced this issue Oct 24, 2023
@mganss
Fix #483
619c8af
@mganss
Copy link
Owner

mganss commented Oct 24, 2023

I was overeager in CSS escaping > also in addition to < (which is necessary for 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 }.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mganss @jasonwnz