Skip to content

Commit

Permalink
recursive invocation to protect against fb55/htmlparser2#105
Browse files Browse the repository at this point in the history
  • Loading branch information
Tom Boutell committed Oct 14, 2014
1 parent f5f61dd commit 762fbc7
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 5 deletions.
4 changes: 4 additions & 0 deletions README.md
Expand Up @@ -180,6 +180,10 @@ sanitizeHtml(

## Changelog

1.4.3: invokes itself recursively until the markup stops changing to guard against [this issue](https://github.com/fb55/htmlparser2/issues/105). Bump to htmlparser2 version 3.7.x.

1.4.1, 1.4.2: more tests.

1.4.0: ability to allow all attributes or tags through by setting `allowedAttributes` and/or `allowedTags` to false. Thanks to Anand Thakker.

1.3.0: `attribs` now available on frames passed to exclusive filter.
Expand Down
22 changes: 19 additions & 3 deletions index.js
Expand Up @@ -4,7 +4,11 @@ var he = require('he');

module.exports = sanitizeHtml;

function sanitizeHtml(html, options) {
// Ignore the _recursing flag; it's there for recursive
// invocation as a guard against this exploit:
// https://github.com/fb55/htmlparser2/issues/105

function sanitizeHtml(html, options, _recursing) {
var result = '';

function Frame(tag, attribs) {
Expand Down Expand Up @@ -87,8 +91,8 @@ function sanitizeHtml(html, options) {
var skipText = false;
var parser = new htmlparser.Parser({
onopentag: function(name, attribs) {
var frame = new Frame(name, attribs);
stack.push(frame);
var frame = new Frame(name, attribs);
stack.push(frame);

var skip = false;
if (_.has(transformTagsMap, name)) {
Expand Down Expand Up @@ -198,6 +202,18 @@ function sanitizeHtml(html, options) {
});
parser.write(html);
parser.end();

// Invoke recursively until we stop finding
// clever little nesting exploits
if (!_recursing) {
while (true) {
var newResult = sanitizeHtml(result, options, true);
if (newResult === result) {
return result;
}
result = newResult;
}
}
return result;

function escapeHtml(s) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -22,7 +22,7 @@
"license": "MIT",
"dependencies": {
"he": "~0.4.1",
"htmlparser2": "~3.3.0",
"htmlparser2": "3.7.x",
"lodash": "2.4.x"
}
}
17 changes: 16 additions & 1 deletion test/test.js
Expand Up @@ -281,6 +281,21 @@ describe('sanitizeHtml', function() {
}
),
''
)
);
});
it('should not be faked out by double <', function() {
assert.equal(
sanitizeHtml('<<img src="javascript:evil"/>img src="javascript:evil"/>'
),
''
);
// I don't love what I get back here obviously, but
// it is not an attack vector, although it might be parsed
// by some browsers as containing an unbalanced close tag.
assert.equal(
sanitizeHtml('<<a href="javascript:evil"/>a href="javascript:evil"/>'
),
'<<a>a href="javascript:evil"/></a>'
);
});
});

0 comments on commit 762fbc7

Please sign in to comment.