Skip to content

Commit

Permalink
Merge pull request #460 from apostrophecms/iframe-validation-redux
Browse files Browse the repository at this point in the history
new and interesting iframe validation exploits
  • Loading branch information
boutell authored Jan 26, 2021
2 parents bff6d9f + 5395e36 commit 6012524
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 [Checkmarx](https://www.checkmarx.com/) 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 [Checkmarx](https://www.checkmarx.com/) 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 6012524

Please sign in to comment.