Skip to content

Commit

Permalink
new and interesting iframe validation exploits
Browse files Browse the repository at this point in the history
  • Loading branch information
boutell committed Jan 26, 2021
1 parent b77e1d9 commit 54851d0
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 2 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# Changelog

## 2.3.2 (2021-01-26):
- Additional fixes for iframe validation exploits. Prevent exploits based on browsers' tolerance of the use of "\" rather than "/" and the presence of whitespace at this point in the URL. Thanks to Ron Masas of <a href="https://www.checkmarx.com/">Checkmarx</a> for pointing out the issue and writing unit tests.

## 2.3.1 (2021-01-22):
- Uses the standard WHATWG URL parser to stop IDNA (Internationalized Domain Name) attacks on the iframe hostname validator. Thanks to Ron Masas of Checkmarx for pointing out the issue and suggesting the use of the WHATWG parser.
- Uses the standard WHATWG URL parser to stop IDNA (Internationalized Domain Name) attacks on the iframe hostname validator. Thanks to Ron Masas of <a href="https://www.checkmarx.com/">Checkmarx</a> for pointing out the issue and suggesting the use of the WHATWG parser.

## 2.3.0 (2020-12-16):
- Upgrades `htmlparser2` to new major version `^6.0.0`. Thanks to [Bogdan Chadkin](https://github.com/TrySound) for the contribution.
Expand Down
4 changes: 4 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,10 @@ function sanitizeHtml(html, options, _recursing) {
if (name === 'iframe' && a === 'src') {
let allowed = true;
try {
// Chrome accepts \ as a substitute for / in the // at the
// start of a URL, so rewrite accordingly to prevent exploit.
// Also drop any whitespace at that point in the URL
value = value.replace(/^(\w+:)?\s*[\\/]\s*[\\/]/, '$1//');
if (value.startsWith('relative:')) {
// An attempt to exploit our workaround for base URLs being
// mandatory for relative URL validation in the WHATWG
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "sanitize-html",
"version": "2.3.1",
"version": "2.3.2",
"description": "Clean up user-submitted HTML, preserving whitelisted elements and whitelisted attributes on a per-element basis",
"sideEffects": false,
"main": "index.js",
Expand Down
56 changes: 56 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1269,4 +1269,60 @@ describe('sanitizeHtml', function() {
'<iframe></iframe>'
);
});
it('Should prevent hostname bypass using protocol-relative src', function () {
assert.strictEqual(
sanitizeHtml('<iframe src="/\\example.com"></iframe>', {
allowedTags: ['iframe'],
allowedAttributes: {
iframe: ['src']
},
allowedIframeHostnames: ["www.youtube.com"],
allowIframeRelativeUrls: true
}), '<iframe></iframe>'
);
assert.strictEqual(
sanitizeHtml('<iframe src="\\/example.com"></iframe>', {
allowedTags: ['iframe'],
allowedAttributes: {
iframe: ['src']
},
allowedIframeHostnames: ["www.youtube.com"],
allowIframeRelativeUrls: true
}), '<iframe></iframe>'
);
const linefeed = decodeURIComponent("%0A");
assert.strictEqual(
sanitizeHtml('<iframe src="/'+linefeed+'\\example.com"></iframe>', {
allowedTags: ['iframe'],
allowedAttributes: {
iframe: ['src']
},
allowedIframeHostnames: ["www.youtube.com"],
allowIframeRelativeUrls: true
}), '<iframe></iframe>'
);
const creturn = decodeURIComponent("%0D");
assert.strictEqual(
sanitizeHtml('<iframe src="/'+creturn+'\\example.com"></iframe>', {
allowedTags: ['iframe'],
allowedAttributes: {
iframe: ['src']
},
allowedIframeHostnames: ["www.youtube.com"],
allowIframeRelativeUrls: true
}), '<iframe></iframe>'
);
const tab = decodeURIComponent("%09");
assert.strictEqual(
sanitizeHtml('<iframe src="/'+tab+'\\example.com"></iframe>', {
allowedTags: ['iframe'],
allowedAttributes: {
iframe: ['src']
},
allowedIframeHostnames: ["www.youtube.com"],
allowIframeRelativeUrls: true
}), '<iframe></iframe>'
);
});

});

0 comments on commit 54851d0

Please sign in to comment.