Module:
Name: html-janitor
Version: 2.0.2
Summary:
Arbitrary HTML can pass the sanitization process, which can be unexpected and dangerous (XSS) in case user-controlled input is passed to the clean function.
Description:
Proof of concept:
var myJanitor = new HTMLJanitor({tags:{p:{}}});
var cleanHtml = myJanitor.clean("<form><object onmouseover=alert(document.domain) name=_sanitized></object></form>")
console.log(cleanHtml) // logs: <form><object onmouseover=alert(document.domain) name=_sanitized></object></form>
The following check can be leveraged to bypass the whole sanitization process:
do {
// Ignore nodes that have already been sanitized
if (node._sanitized) {
continue;
}
...
As node
is the first child in the created tree walker (i.e. in this case the <form>
tag) node._sanitized
will point to the inner <object>
and the check passes.
To learn more about DOM clobbering see: https://www.youtube.com/watch?v=5W-zGBKvLxk (by Mario Heiderich)
Recommendation:
It should be enough to set node._sanitized
to false
every time a new node is being processed.
Note that I previously reported this issue at https://github.com/guardian/html-janitor/issues/35
Given the module’s description I would assume it should be used to prevent XSS vulnerabilities. This is currently a very dangerous assumption given that the whole sanitization process can be bypassed.
Note that the author might have never intended to feed untrusted data into the clean() function. In that case this is just a regular issue. Furthermore, the fact that untrusted data is unexpected should be at least mentioned in the documentation, because other developers most certainly will use the package in such scenarios.