Repository navigation
String#unescapeHTML() decodes entities after stripping tags, reintroducing markup #371
Description
Activity
Adding some cross-references after going through the existing open issues, so this is not read in isolation:
- This sits in the same area as CVE-2020-27511, which already covers
String#stripTags/String#unescapeHTML(see also CVE-2020-27511 #355 and OWASP dependency-check (Retire.js) flags every fork of 1.7.3 for CVE-2020-27511, even patched ones — request a tagged 1.7.4 / version bump #368). I am not claiming a new CVE here. - unescapeHTML() is casesensetive #251 covers a different aspect of the same method (entity case sensitivity,
<vs<).
The specific point I am raising is narrower and, as far as I can tell, not captured by either of those: the order of operations.
unescapeHTML()strips tags first and then decodes entities, so encoded markup that is not yet a tag at strip time gets decoded back into live markup afterwards. Example:'<img src=x onerror=alert(1)>'.unescapeHTML(); // stripTags() leaves the entity text alone (no real tag yet), // then the decode step yields a live tag: // => '<img src=x onerror=alert(1)>'
So anything that treats the output of
unescapeHTML()as tag-free before inserting it into the DOM can end up reintroducing executable markup. Decoding first and then stripping (or a single normalization pass) would avoid this. Either way it would help to document that the result is not safe to insert as HTML.- This sits in the same area as CVE-2020-27511, which already covers
String#unescapeHTML()callsstripTags()first and then decodes entities. Because the decode runs after the strip, encoded markup that survives stripping (since it is not a real tag at that point) gets turned back into live markup. Any code that assumes the output is tag-free will be wrong.Current implementation (around line 439 of
src/prototype/lang/string.js):Reproduction:
If a developer relies on
unescapeHTML()to produce safe, tag-free text before inserting it into the page, the decode step reintroduces executable markup, which is a path to XSS.Suggested fix: decode entities first and then strip, or use a single normalization pass that does not leave decoded markup behind. It would also help to document that the result is not safe to insert into the DOM as HTML.
Refs: CWE-79, CWE-116, OWASP ASVS V5.3.3.