Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 8f31f1f

Browse files
petebacondarwinpetebacondarwin
committed
fix($sanitize): use appropriate inert document strategy for Firefox and 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.
1 parent cf84fcf commit 8f31f1f

File tree

2 files changed

+89
-16
lines changed

2 files changed

+89
-16
lines changed

‎src/ngSanitize/sanitize.js‎

Lines changed: 75 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -313,16 +313,78 @@ function $SanitizeProvider() {
313313
return obj;
314314
}
315315

316-
var inertBodyElement = (function(window) {
317-
var doc;
318-
if (window.document && window.document.implementation) {
319-
doc = window.document.implementation.createHTMLDocument('inert');
316+
/**
317+
* Create an inert document that contains the dirty HTML that needs sanitizing
318+
* Depending upon browser support we use one of three strategies for doing this.
319+
* Support: Safari 10.x -> XHR strategy
320+
* Support: Firefox -> DomParser strategy
321+
*/
322+
var getInertBodyElement /* function(html: string): HTMLBodyElement */ = (function(window, document) {
323+
var inertDocument;
324+
if (document && document.implementation) {
325+
inertDocument = document.implementation.createHTMLDocument('inert');
320326
} else {
321327
throw $sanitizeMinErr('noinert', 'Can\'t create an inert html document');
322328
}
323-
var docElement = doc.documentElement || doc.getDocumentElement();
324-
return docElement.getElementsByTagName('body')[0];
325-
})(window);
329+
var inertBodyElement = (inertDocument.documentElement || inertDocument.getDocumentElement()).querySelector('body');
330+
331+
// Check for the Safari 10.1 bug - which allows JS to run inside the SVG G element
332+
inertBodyElement.innerHTML = '<svg><g onload="this.parentNode.remove()"></g></svg>';
333+
if (!inertBodyElement.querySelector('svg')) {
334+
return getInertBodyElement_XHR;
335+
} else {
336+
// Check for the Firefox bug - which prevents the inner img JS from being sanitized
337+
inertBodyElement.innerHTML = '<svg><p><style><img src="</style><img src=x onerror=alert(1)//">';
338+
if (inertBodyElement.querySelector('svg img')) {
339+
return getInertBodyElement_DOMParser;
340+
} else {
341+
return getInertBodyElement_InertDocument;
342+
}
343+
}
344+
345+
function getInertBodyElement_XHR(html) {
346+
// We add this dummy element to ensure that the rest of the content is parsed as expected
347+
// e.g. leading whitespace is maintained and tags like `<meta>` do not get hoisted to the `<head>` tag.
348+
html = '<remove></remove>' + html;
349+
try {
350+
html = encodeURI(html);
351+
} catch (e) {
352+
return undefined;
353+
}
354+
var xhr = new window.XMLHttpRequest();
355+
xhr.responseType = 'document';
356+
xhr.open('GET', 'data:text/html;charset=utf-8,' + html, false);
357+
xhr.send(null);
358+
var body = xhr.response.body;
359+
body.firstChild.remove();
360+
return body;
361+
}
362+
363+
function getInertBodyElement_DOMParser(html) {
364+
// We add this dummy element to ensure that the rest of the content is parsed as expected
365+
// e.g. leading whitespace is maintained and tags like `<meta>` do not get hoisted to the `<head>` tag.
366+
html = '<remove></remove>' + html;
367+
try {
368+
var body = new window.DOMParser().parseFromString(html, 'text/html').body;
369+
body.firstChild.remove();
370+
return body;
371+
} catch (e) {
372+
return undefined;
373+
}
374+
}
375+
376+
function getInertBodyElement_InertDocument(html) {
377+
inertBodyElement.innerHTML = html;
378+
379+
// Support: IE 9-11 only
380+
// strip custom-namespaced attributes on IE<=11
381+
if (document.documentMode) {
382+
stripCustomNsAttrs(inertBodyElement);
383+
}
384+
385+
return inertBodyElement;
386+
}
387+
})(window, window.document);
326388

327389
/**
328390
* @example
@@ -342,7 +404,9 @@ function $SanitizeProvider() {
342404
} else if (typeof html !== 'string') {
343405
html = '' + html;
344406
}
345-
inertBodyElement.innerHTML = html;
407+
408+
var inertBodyElement = getInertBodyElement(html);
409+
if (!inertBodyElement) return '';
346410

347411
//mXSS protection
348412
var mXSSAttempts = 5;
@@ -352,13 +416,9 @@ function $SanitizeProvider() {
352416
}
353417
mXSSAttempts--;
354418

355-
// Support: IE 9-11 only
356-
// strip custom-namespaced attributes on IE<=11
357-
if (window.document.documentMode) {
358-
stripCustomNsAttrs(inertBodyElement);
359-
}
360-
html = inertBodyElement.innerHTML; //trigger mXSS
361-
inertBodyElement.innerHTML = html;
419+
// trigger mXSS if it is going to happen by reading and writing the innerHTML
420+
html = inertBodyElement.innerHTML;
421+
inertBodyElement = getInertBodyElement(html);
362422
} while (html !== inertBodyElement.innerHTML);
363423

364424
var node = inertBodyElement.firstChild;

‎test/ngSanitize/sanitizeSpec.js‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ describe('HTML', function() {
4949
comment = comment_;
5050
}
5151
};
52+
// Trigger the $sanitizer provider to execute, which initializes the `htmlParser` function.
53+
inject(function($sanitize) {});
5254
});
5355

5456
it('should not parse comments', function() {
@@ -266,14 +268,25 @@ describe('HTML', function() {
266268
});
267269
});
268270

271+
// See https://github.com/cure53/DOMPurify/blob/a992d3a75031cb8bb032e5ea8399ba972bdf9a65/src/purify.js#L439-L449
272+
it('should not allow JavaScript execution when creating inert document', inject(function($sanitize) {
273+
var doc = $sanitize('<svg><g onload="window.xxx = 100"></g></svg>');
274+
expect(window.xxx).toBe(undefined);
275+
delete window.xxx;
276+
}));
277+
278+
// See https://github.com/cure53/DOMPurify/releases/tag/0.6.7
279+
it('should not allow JavaScript hidden in badly formed HTML to get through sanitization (Firefox bug)', inject(function($sanitize) {
280+
var doc = $sanitize('<svg><p><style><img src="</style><img src=x onerror=alert(1)//">');
281+
expect(doc).toEqual('<p><img src="x"></p>');
282+
}));
269283

270284
describe('SVG support', function() {
271285

272286
beforeEach(module(function($sanitizeProvider) {
273287
$sanitizeProvider.enableSvg(true);
274288
}));
275289

276-
277290
it('should accept SVG tags', function() {
278291
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>')
279292
.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>',

0 commit comments

Comments
 (0)