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

Commit 769306d

Browse files
fix(sanitizeUri): sanitize URIs that contain IDEOGRAPHIC SPACE chars
Chrome 62 was not sanitizing dangerous URLs containing JavaScript, if they started with these "whitespace" characters. --- Browsers convert `&#12288;javascript:alert(1)` as an attribute value to `javascript:alert(1)`. So the sanitizer gets the second string and is able to strip the javascript from the attribute. But Chrome (<62) only did this after you read it and wrote it back again. So we added a bit of code that tried to get Chrome to do its conversion before sanitizing it: https://github.com/angular/angular.js/blob/817ac567/src/ngSanitize/sanitize.js#L406-L417 I believe that Chrome 62 now does not do this conversion any more, instead it just leaves the attribute value alone, whatever you do to it. This fix uses `trim()` to remove the problematic whitespace before sanitizing, which appears to solve the problem. Closes #16288
1 parent 817ac56 commit 769306d

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)