Skip to content

Commit

Permalink
fix($sanitize): use appropriate inert document strategy for Firefox a…
Browse files Browse the repository at this point in the history
…nd Safari

Both Firefox and Safari are vulnerable to XSS if we use an inert document
created via `document.implementation.createHTMLDocument()`.

Now we check for those vulnerabilities and then use a DOMParser or XHR
strategy if needed.

Thanks to @cure53 for the heads up on this issue.
  • Loading branch information
petebacondarwin committed Jun 5, 2017
1 parent e65928e commit 7673ca7
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 16 deletions.
90 changes: 75 additions & 15 deletions src/ngSanitize/sanitize.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,16 +313,78 @@ function $SanitizeProvider() {
return obj;
}

var inertBodyElement = (function(window) {
var doc;
if (window.document && window.document.implementation) {
doc = window.document.implementation.createHTMLDocument('inert');
/**
* Create an inert document that contains the dirty HTML that needs sanitizing
* Depending upon browser support we use one of three strategies for doing this.
* Support: Safari 10.x -> XHR strategy
* Support: Firefox -> DomParser strategy
*/
var getInertBodyElement /* function(html: string): HTMLBodyElement */ = (function(window, document) {
var inertDocument;
if (document && document.implementation) {
inertDocument = document.implementation.createHTMLDocument('inert');
} else {
throw $sanitizeMinErr('noinert', 'Can\'t create an inert html document');
}
var docElement = doc.documentElement || doc.getDocumentElement();
return docElement.getElementsByTagName('body')[0];
})(window);
var inertBodyElement = (inertDocument.documentElement || inertDocument.getDocumentElement()).querySelector('body');

// Check for the Safari 10.1 bug - which allows JS to run inside the SVG G element
inertBodyElement.innerHTML = '<svg><g onload="this.parentNode.remove()"></g></svg>';
if (!inertBodyElement.querySelector('svg')) {
return getInertBodyElement_XHR;
} else {
// Check for the Firefox bug - which prevents the inner img JS from being sanitized
inertBodyElement.innerHTML = '<svg><p><style><img src="</style><img src=x onerror=alert(1)//">';
if (inertBodyElement.querySelector('svg img')) {
return getInertBodyElement_DOMParser;
} else {
return getInertBodyElement_InertDocument;
}
}

function getInertBodyElement_XHR(html) {
// We add this dummy element to ensure that the rest of the content is parsed as expected
// e.g. leading whitespace is maintained and tags like `<meta>` do not get hoisted to the `<head>` tag.
html = '<remove></remove>' + html;
try {
html = encodeURI(html);
} catch (e) {
return undefined;
}
var xhr = new window.XMLHttpRequest();
xhr.responseType = 'document';
xhr.open('GET', 'data:text/html;charset=utf-8,' + html, false);
xhr.send(null);
var body = xhr.response.body;
body.firstChild.remove();
return body;
}

function getInertBodyElement_DOMParser(html) {
// We add this dummy element to ensure that the rest of the content is parsed as expected
// e.g. leading whitespace is maintained and tags like `<meta>` do not get hoisted to the `<head>` tag.
html = '<remove></remove>' + html;
try {
var body = new window.DOMParser().parseFromString(html, 'text/html').body;
body.firstChild.remove();
return body;
} catch (e) {
return undefined;
}
}

function getInertBodyElement_InertDocument(html) {
inertBodyElement.innerHTML = html;

// Support: IE 9-11 only
// strip custom-namespaced attributes on IE<=11
if (document.documentMode) {
stripCustomNsAttrs(inertBodyElement);
}

return inertBodyElement;
}
})(window, window.document);

/**
* @example
Expand All @@ -342,7 +404,9 @@ function $SanitizeProvider() {
} else if (typeof html !== 'string') {
html = '' + html;
}
inertBodyElement.innerHTML = html;

var inertBodyElement = getInertBodyElement(html);
if (!inertBodyElement) return '';

//mXSS protection
var mXSSAttempts = 5;
Expand All @@ -352,13 +416,9 @@ function $SanitizeProvider() {
}
mXSSAttempts--;

// Support: IE 9-11 only
// strip custom-namespaced attributes on IE<=11
if (window.document.documentMode) {
stripCustomNsAttrs(inertBodyElement);
}
html = inertBodyElement.innerHTML; //trigger mXSS
inertBodyElement.innerHTML = html;
// trigger mXSS if it is going to happen by reading and writing the innerHTML
html = inertBodyElement.innerHTML;
inertBodyElement = getInertBodyElement(html);
} while (html !== inertBodyElement.innerHTML);

var node = inertBodyElement.firstChild;
Expand Down
15 changes: 14 additions & 1 deletion test/ngSanitize/sanitizeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ describe('HTML', function() {
comment = comment_;
}
};
// Trigger the $sanitizer provider to execute, which initializes the `htmlParser` function.
inject(function($sanitize) {});
});

it('should not parse comments', function() {
Expand Down Expand Up @@ -266,14 +268,25 @@ describe('HTML', function() {
});
});

// See https://github.com/cure53/DOMPurify/blob/a992d3a75031cb8bb032e5ea8399ba972bdf9a65/src/purify.js#L439-L449
it('should not allow JavaScript execution when creating inert document', inject(function($sanitize) {
var doc = $sanitize('<svg><g onload="window.xxx = 100"></g></svg>');
expect(window.xxx).toBe(undefined);
delete window.xxx;
}));

// See https://github.com/cure53/DOMPurify/releases/tag/0.6.7
it('should not allow JavaScript hidden in badly formed HTML to get through sanitization (Firefox bug)', inject(function($sanitize) {
var doc = $sanitize('<svg><p><style><img src="</style><img src=x onerror=alert(1)//">');
expect(doc).toEqual('<p><img src="x"></p>');
}));

describe('SVG support', function() {

beforeEach(module(function($sanitizeProvider) {
$sanitizeProvider.enableSvg(true);
}));


it('should accept SVG tags', function() {
expectHTML('<svg width="400px" height="150px" xmlns="http://www.w3.org/2000/svg"><circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red"></svg>')
.toBeOneOf('<svg width="400px" height="150px" xmlns="http://www.w3.org/2000/svg"><circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red"></circle></svg>',
Expand Down

0 comments on commit 7673ca7

Please sign in to comment.