Skip to content

Commit 667db46

Browse files
fix(sanitizeUri): sanitize URIs that contain IDEOGRAPHIC SPACE chars
Browsers mutate attributes values such as `&angular#12288;javascript:alert(1)` when they are written to the DOM via `innerHTML` in various vendor specific ways. In Chrome (<62), this mutation removed the preceding "whitespace" resulting in a value that could end up being executed as JavaScript. Here is an example of what could happen: https://plnkr.co/edit/Y6EsbsuDgd18YTn1oARu?p=preview If you run that in Chrome 61 you will get a dialog box pop up. There is background here: http://www.nds.rub.de/media/emma/veroeffentlichungen/2013/12/10/mXSS-CCS13.pdf The sanitizer has a bit of code that triggers this mutation on an inert piece of DOM, before we try to sanitize it: https://github.com/angular/angular.js/blob/817ac567/src/ngSanitize/sanitize.js#L406-L417 Chrome 62 does not appear to mutate this particular string any more, instead it just leaves the "whitespace" in place. This probably means that Chrome 62 is no longer vulnerable to this specific attack vector; but there may be other mutating strings that we haven't found, which are vulnerable. Since we are leaving the mXSS check in place, the sanitizer should still be immune to any strings that try to utilise this attack vector. This commit uses `trim()` to remove the IDEOGRAPHIC SPACE "whitespace" before sanitizing, which allows us to expose this mXSS test to all browsers rather than just Chrome. Closes angular#16288
1 parent 2c9c3a0 commit 667db46

File tree

2 files changed

+4
-6
lines changed

2 files changed

+4
-6
lines changed

src/ng/sanitizeUri.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ function $$SanitizeUriProvider() {
6262
return function sanitizeUri(uri, isImage) {
6363
var regex = isImage ? imgSrcSanitizationWhitelist : aHrefSanitizationWhitelist;
6464
var normalizedVal;
65-
normalizedVal = urlResolve(uri).href;
65+
normalizedVal = urlResolve(uri && uri.trim()).href;
6666
if (normalizedVal !== '' && !normalizedVal.match(regex)) {
6767
return 'unsafe:' + normalizedVal;
6868
}

test/ngSanitize/sanitizeSpec.js

+3-5
Original file line numberDiff line numberDiff line change
@@ -237,11 +237,9 @@ describe('HTML', function() {
237237
.toEqual('');
238238
});
239239

240-
if (isChrome) {
241-
it('should prevent mXSS attacks', function() {
242-
expectHTML('<a href="&#x3000;javascript:alert(1)">CLICKME</a>').toBe('<a>CLICKME</a>');
243-
});
244-
}
240+
it('should prevent mXSS attacks', function() {
241+
expectHTML('<a href="&#x3000;javascript:alert(1)">CLICKME</a>').toBe('<a>CLICKME</a>');
242+
});
245243

246244
it('should strip html comments', function() {
247245
expectHTML('<!-- comment 1 --><p>text1<!-- comment 2 -->text2</p><!-- comment 3 -->')

0 commit comments

Comments
 (0)