Skip to content

Commit

Permalink
stop IDNA iframe attacks
Browse files Browse the repository at this point in the history
  • Loading branch information
boutell committed Jan 22, 2021
1 parent 7229906 commit ca4b62a
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 6 deletions.
23 changes: 17 additions & 6 deletions index.js
Expand Up @@ -5,7 +5,6 @@ const { isPlainObject } = require('is-plain-object');
const deepmerge = require('deepmerge');
const parseSrcset = require('parse-srcset');
const { parse: postcssParse } = require('postcss');
const url = require('url');
// Tags that can conceivably represent stand-alone media.
const mediaTags = [
'img', 'audio', 'video', 'picture', 'svg',
Expand Down Expand Up @@ -305,12 +304,24 @@ function sanitizeHtml(html, options, _recursing) {
if (name === 'iframe' && a === 'src') {
let allowed = true;
try {
if (value.startsWith('relative:')) {
// An attempt to exploit our workaround for base URLs being
// mandatory for relative URL validation in the WHATWG
// URL parser, reject it
throw new Error('relative: exploit attempt');
}
// naughtyHref is in charge of whether protocol relative URLs
// are cool. We should just accept them
// TODO: Replace deprecated `url.parse`
// eslint-disable-next-line node/no-deprecated-api
parsed = url.parse(value, false, true);
const isRelativeUrl = parsed && parsed.host === null && parsed.protocol === null;
// are cool. Here we are concerned just with allowed hostnames and
// whether to allow relative URLs.
//
// Build a placeholder "base URL" against which any reasonable
// relative URL may be parsed successfully
let base = 'relative://relative-site';
for (let i = 0; (i < 100); i++) {
base += `/${i}`;
}
const parsed = new URL(value, base);
const isRelativeUrl = parsed && parsed.hostname === 'relative-site' && parsed.protocol === 'relative:';
if (isRelativeUrl) {
// default value of allowIframeRelativeUrls is true
// unless allowedIframeHostnames or allowedIframeDomains specified
Expand Down
59 changes: 59 additions & 0 deletions test/test.js
Expand Up @@ -1210,4 +1210,63 @@ describe('sanitizeHtml', function() {
'<div><div><div><div><div><div>nested text</div></div></div></div></div></div>'
);
});
it('should not allow simple append attacks on iframe hostname validation', function() {
assert.equal(
sanitizeHtml('<iframe src=//www.youtube.comissocool>', {
allowedTags: [ 'iframe' ],
allowedAttributes: {
iframe: [ 'src' ]
},
allowedIframeHostnames: [ 'www.youtube.com' ]
}),
'<iframe></iframe>'
);
});
it('should not allow IDNA (Internationalized Domain Name) iframe validation bypass attacks', function() {
assert.equal(
sanitizeHtml('<iframe src=//www.youtube.com%C3%9E.93.184.216.34.nip.io>', {
allowedTags: [ 'iframe' ],
allowedAttributes: {
iframe: [ 'src' ]
},
allowedIframeHostnames: [ 'www.youtube.com' ]
}),
'<iframe></iframe>'
);
});
it('should parse path-rooted relative URLs sensibly', function() {
assert.equal(
sanitizeHtml('<a href="/foo"></a>'),
'<a href="/foo"></a>'
);
});
it('should parse bare relative URLs sensibly', function() {
assert.equal(
sanitizeHtml('<a href="foo"></a>'),
'<a href="foo"></a>'
);
});
it('should parse ../ relative URLs sensibly', function() {
assert.equal(
sanitizeHtml('<a href="../../foo"></a>'),
'<a href="../../foo"></a>'
);
});
it('should parse protocol relative URLs sensibly', function() {
assert.equal(
sanitizeHtml('<a href="//foo.com/foo"></a>'),
'<a href="//foo.com/foo"></a>'
);
});
it('should reject attempts to hack our use of a relative: protocol in our test base URL', function() {
assert.equal(
sanitizeHtml('<iframe src="relative://relative-test/aha">', {
allowedTags: [ 'iframe' ],
allowedAttributes: {
iframe: [ 'src' ]
}
}),
'<iframe></iframe>'
);
});
});

0 comments on commit ca4b62a

Please sign in to comment.