From 54851d0df96ba4016e203734e4432578017a5325 Mon Sep 17 00:00:00 2001 From: Tom Boutell Date: Tue, 26 Jan 2021 08:59:23 -0500 Subject: [PATCH 1/3] new and interesting iframe validation exploits --- CHANGELOG.md | 5 ++++- index.js | 4 ++++ package.json | 2 +- test/test.js | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4861e0a..eea4fa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 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 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. diff --git a/index.js b/index.js index d37ee99..fe4dd9e 100644 --- a/index.js +++ b/index.js @@ -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 diff --git a/package.json b/package.json index 0d390a1..52eeca4 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/test/test.js b/test/test.js index 634ada4..6f5583e 100644 --- a/test/test.js +++ b/test/test.js @@ -1269,4 +1269,60 @@ describe('sanitizeHtml', function() { '' ); }); + it('Should prevent hostname bypass using protocol-relative src', function () { + assert.strictEqual( + sanitizeHtml('', { + allowedTags: ['iframe'], + allowedAttributes: { + iframe: ['src'] + }, + allowedIframeHostnames: ["www.youtube.com"], + allowIframeRelativeUrls: true + }), '' + ); + assert.strictEqual( + sanitizeHtml('', { + allowedTags: ['iframe'], + allowedAttributes: { + iframe: ['src'] + }, + allowedIframeHostnames: ["www.youtube.com"], + allowIframeRelativeUrls: true + }), '' + ); + const linefeed = decodeURIComponent("%0A"); + assert.strictEqual( + sanitizeHtml('', { + allowedTags: ['iframe'], + allowedAttributes: { + iframe: ['src'] + }, + allowedIframeHostnames: ["www.youtube.com"], + allowIframeRelativeUrls: true + }), '' + ); + const creturn = decodeURIComponent("%0D"); + assert.strictEqual( + sanitizeHtml('', { + allowedTags: ['iframe'], + allowedAttributes: { + iframe: ['src'] + }, + allowedIframeHostnames: ["www.youtube.com"], + allowIframeRelativeUrls: true + }), '' + ); + const tab = decodeURIComponent("%09"); + assert.strictEqual( + sanitizeHtml('', { + allowedTags: ['iframe'], + allowedAttributes: { + iframe: ['src'] + }, + allowedIframeHostnames: ["www.youtube.com"], + allowIframeRelativeUrls: true + }), '' + ); + }); + }); From 1ecf30f623b1da5eebe21f3a60594714730d6bce Mon Sep 17 00:00:00 2001 From: Tom Boutell Date: Tue, 26 Jan 2021 09:13:55 -0500 Subject: [PATCH 2/3] pass eslint --- test/test.js | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/test.js b/test/test.js index 6f5583e..a0ffbe2 100644 --- a/test/test.js +++ b/test/test.js @@ -1272,54 +1272,54 @@ describe('sanitizeHtml', function() { it('Should prevent hostname bypass using protocol-relative src', function () { assert.strictEqual( sanitizeHtml('', { - allowedTags: ['iframe'], + allowedTags: [ 'iframe' ], allowedAttributes: { - iframe: ['src'] + iframe: [ 'src' ] }, - allowedIframeHostnames: ["www.youtube.com"], + allowedIframeHostnames: [ 'www.youtube.com' ], allowIframeRelativeUrls: true }), '' ); assert.strictEqual( sanitizeHtml('', { - allowedTags: ['iframe'], + allowedTags: [ 'iframe' ], allowedAttributes: { - iframe: ['src'] + iframe: [ 'src' ] }, - allowedIframeHostnames: ["www.youtube.com"], + allowedIframeHostnames: [ 'www.youtube.com' ], allowIframeRelativeUrls: true }), '' ); - const linefeed = decodeURIComponent("%0A"); + const linefeed = decodeURIComponent('%0A'); assert.strictEqual( - sanitizeHtml('', { - allowedTags: ['iframe'], + sanitizeHtml('', { + allowedTags: [ 'iframe' ], allowedAttributes: { - iframe: ['src'] + iframe: [ 'src' ] }, - allowedIframeHostnames: ["www.youtube.com"], + allowedIframeHostnames: [ 'www.youtube.com' ], allowIframeRelativeUrls: true }), '' ); - const creturn = decodeURIComponent("%0D"); + const creturn = decodeURIComponent('%0D'); assert.strictEqual( - sanitizeHtml('', { - allowedTags: ['iframe'], + sanitizeHtml('', { + allowedTags: [ 'iframe' ], allowedAttributes: { - iframe: ['src'] + iframe: [ 'src' ] }, - allowedIframeHostnames: ["www.youtube.com"], + allowedIframeHostnames: [ 'www.youtube.com' ], allowIframeRelativeUrls: true }), '' ); - const tab = decodeURIComponent("%09"); + const tab = decodeURIComponent('%09'); assert.strictEqual( - sanitizeHtml('', { - allowedTags: ['iframe'], + sanitizeHtml('', { + allowedTags: [ 'iframe' ], allowedAttributes: { - iframe: ['src'] + iframe: [ 'src' ] }, - allowedIframeHostnames: ["www.youtube.com"], + allowedIframeHostnames: [ 'www.youtube.com' ], allowIframeRelativeUrls: true }), '' ); From 5395e36fcf75eb09329aa7e60435896bbe443b95 Mon Sep 17 00:00:00 2001 From: Tom Boutell Date: Tue, 26 Jan 2021 15:32:20 -0500 Subject: [PATCH 3/3] markdown --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eea4fa4..fcb4c8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +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 for pointing out the issue and writing unit tests. +- 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.