4 ms·
> So DOMPurify has nothing to do here, and returns a serialized HTML: <form><math><mtext><form><mglyph><style></math><img src onerror=alert(1)></style></mg
by kijin 6y ago
> So DOMPurify has nothing to do here, and returns a serialized HTML:
<form><math><mtext><form><mglyph><style></math><img src onerror=alert(1)></style></mglyph></form></mtext></math></form>
This seems to be the wrong part. If "</math><img src onerror=alert(1)>" was interpreted as part of a #text node, it should have been serialized into "</math><img src onerror=alert(1)>" so that it becomes harmless in any subsequent round-trip.
- thaumasiotes 6y agoI think you need to do something more than that, or round trips will give you text that looks like &amp;amp;lt;img src onerror=alert(1)&amp;amp;gt;
- kijin 6y agoOf course, special characters that are already escaped, i.e. already valid in a text node, don't need to be escaped again. Even PHP provided a function to do that 20 years ago. It should be relatively straightforward in any modern language with regexp support.
- thaumasiotes 6y ago> Of course, special characters that are already escaped, i.e. already valid in a text node, don't need to be escaped again. But this isn't true at all. You could have text that's supposed to look like "<span>" -- just look at the discussion we're having right now. And that text needs to be rendered as "&lt;span&gt;", but it also needs not to be rendered as "&amp;lt;span&amp;gt;". You have to keep track of what you've done to it.
- kijin 6y agoAny decent WYSIWYG editor (which I assume your data is coming from if you're trying to sanitize arbitrary HTML) will render "<span>" as "&lt;span&gt;" in the first place. The sanitizer doesn't need to do anything to it, as the markup is already valid.
- thaumasiotes 6y agoYou've managed to miss the whole point of the discussion thread and the article above it. Your data is coming from a user. You need to change "<img src onerror=alert(1)>" to "<img src onerror=alert(1)>". You will not be able to do this if your approach is "doesn't matter; we'll just trust whatever we get".
- sergeykish 6y agoThat's XHTML encoding. It would survive round trip if host document was XHTML.
- minitech 6y agoIt (thinks it) can’t do that because <style> introduces a special parsing mode where those character references would be interpreted literally.
- kijin 6y agoThen it needs to be better aware of what kind of syntax is allowed in CSS, because </math> clearly isn't valid CSS. Sanitizing HTML isn't just about removing unwanted tags and attributes. Assuming you want to preserve some styling, the sanitizer needs to be aware of the spec for not only HTML but also CSS, JS, SVG, and whatnot. It must ensure that it never emits output that would be invalid in any of those contexts. (JS is easy because you normally just delete it.) To date, I haven't seen any HTML sanitizer that's as serious about enforcing spec as HTMLPurifier. It strips out everything that isn't in a whitelist and guarantees that the output is syntactically valid. Unfortunately, it's written in PHP, its whitelist is kinda outdated, and nobody seems to want to port it to any other language. But the only alternative is to take dangerous shortcuts.
- missblit 6y agoNot sure strict CSS parsing would have avoided this issue, since you could just surround what the purifier thought was a style text node with /* and */ (which is valid CSS, being a comment)
- securitymb 6y agoEven if comments were stripped (which sometimes happens), you could still put it in the URL or font family name: * { font-family: '</math>...'; }
- kijin 6y agoSanitizers are not required to keep the original content intact. They absolutely can, and should, strip invalid and/or suspicious content from CSS strings.
- 6y ago
- yread 6y agoThere is really no reason to even allow style element in dompurify
- chrismorgan 6y agoNot at all true. As an example, HTML emails may contain <style>, and any webmail client will want to render that, and they’ll definitely want to sanitise the HTML. Fastmail uses DOMPurify for this purpose, augmenting it with rewriting the selectors in style blocks in order to scope them safely. (Fastmail also sponsors the DOMPurify bug bounty—I presume the latest mXSS listed at https://www.fastmail.com/about/bugbounty/ https://www.fastmail.com/about/bugbounty/ corresponds to this thing, and as noted, this issue didn’t affect Fastmail; the reason for that is that Fastmail uses the DOM that DOMPurify returns, rather than problematic and less efficient serialising and deserialising approach.)
- yread 6y agoGood point. I forgot about the abomination that is HTML emails
- chrismorgan 6y agoAnd this is hardly the only justifiable reason for allowing <style>. A couple of other examples that occur to me are platforms allowing full user-generated and -styled pages, and validation tools. Remember with this that each application can choose what it wants to allow or deny. It would be very harmful for DOMPurify to not allow certain sorts of tags just because you can’t immediately think up a reason why you might want to use it.
- sergeykish 6y agoThat's exactly what XHTML does, in XHTML document sample = `<form><math><mtext></form><form><mglyph><style></math><img src onerror=alert(1)>` fragment = DOMPurify.sanitize(sample, {RETURN_DOM_FRAGMENT: true}) body = (new XMLSerializer).serializeToString(fragment) //...<style></math><img src onerror=alert(1)></style>... div.innerHTML = body div.append(fragment) iframe.srcdoc = body It renders nice </math><img src onerror=alert(1)>