-
Notifications
You must be signed in to change notification settings - Fork 18
Description
Not sending a PR for this one because neither do I exactly know what's causing it or do I think it's that big of a deal that it's worth spending a lot of time on debugging, but making a note here anyhow so that it's documented.
In the checkmention project by @kbsriram there's an XSS test that parses differently now compared to version 0.3.x: https://github.com/kbsriram/checkmention/blob/master/src/WEB-INF/checks/xss
More specifically – the 2.0.0 version of this library is now ignoring the final </script> in the <<SCRIPT>alert("XSS4");//<</SCRIPT> code in there and thus makes all of the remaining e-content be treated as content of the script tag and thus drops all of that content from the text. This didn't happen before.
This means that the text output of parsing that XSS file is now:
Clicking this\nshould not cause an alert.\nThis div\nshould not alert.\nTry clicking this link\n<script>alert(\"encoded-xss\")</script>\nand this too.\nMouse over this\nshould not cause an alert. This broken\n should not throw an alert.\n<
When it before was:
Clicking this\nshould not cause an alert.\nThis div\nshould not alert.\nTry clicking this link\n<script>alert(\"encoded-xss\")</script>\nand this too.\nMouse over this\nshould not cause an alert. This broken\n should not throw an alert.\n<alert(\"XSS4\");//\n\nNeither should .\nPlease look at the Owasp XSS prevention cheat sheet for more information.\n\n\nThis note was created on\n\n%%nice_time
When looking at the same content directly in the browser I would say that the former handling was more correct than the current, but not sure what has changed. Cheerio is still the same so must be something outside of Cheerios control?
But – as I said in the beginning – this feels like an edge case that's not really worth spending a lot of time on fixing if it isn't an indicator of something bigger which I don't think it is.