microcosm-cc / bluemonday

bluemonday: a fast golang HTML sanitizer (inspired by the OWASP Java HTML Sanitizer) to scrub user generated content of XSS
https://github.com/microcosm-cc/bluemonday
BSD 3-Clause "New" or "Revised" License
3.12k stars 176 forks source link

Don't write self-closing tag with empty attributes #151

Closed Gusted closed 1 year ago

Gusted commented 1 year ago
buro9 commented 1 year ago

Thanks, this is a good catch and clearly a long-time minor bug.

For the people from Snyk, etc it is not a security bug in that this resolves an issue that wouldn't have led to an XSS.

What this bug is... it's tiny, but some elements are meaningful in HTML without attributes, for example the <hr/> has no attributes and will draw a horizontal rule. But other elements are meaningless without attributes as they result in a no-op... and in those cases where sanitization would've removed all attributes, then the element should also be removed.

This bug meant that a policy that added an element via allowing an attribute... where the attribute was not actually in the input... then the element was still meaningless and should've been removed.

The bug produced a no-op... hence no risk. But still, a bug is a bug and this fix correctly aligns the output with the desired state, which is to remove empty elements that are meaningless without attributes.

Thanks @Gusted for the change.