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

Commit 9532234

Browse files
IgorMinarIgorMinar
committed
fix($compile): sanitize values bound to a[href]
1 parent 5f5d4fe commit 9532234

File tree

3 files changed

+197
-9
lines changed

3 files changed

+197
-9
lines changed

‎src/ng/compile.js

+53-7
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ function $CompileProvider($provide) {
155155
Suffix = 'Directive',
156156
COMMENT_DIRECTIVE_REGEXP = /^\s*directive\:\s*([\d\w\-_]+)\s+(.*)$/,
157157
CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/,
158-
MULTI_ROOT_TEMPLATE_ERROR = 'Template must have exactly one root element. was: ';
158+
MULTI_ROOT_TEMPLATE_ERROR = 'Template must have exactly one root element. was: ',
159+
urlSanitizationWhitelist = /^\s*(https?|ftp|mailto):/;
159160

160161

161162
/**
@@ -209,11 +210,41 @@ function $CompileProvider($provide) {
209210
};
210211

211212

213+
/**
214+
* @ngdoc function
215+
* @name ng.$compileProvider#urlSanitizationWhitelist
216+
* @methodOf ng.$compileProvider
217+
* @function
218+
*
219+
* @description
220+
* Retrieves or overrides the default regular expression that is used for whitelisting of safe
221+
* urls during a[href] sanitization.
222+
*
223+
* The sanitization is a security measure aimed at prevent XSS attacks via html links.
224+
*
225+
* Any url about to be assigned to a[href] via data-binding is first normalized and turned into an
226+
* absolute url. Afterwards the url is matched against the `urlSanitizationWhitelist` regular
227+
* expression. If a match is found the original url is written into the dom. Otherwise the
228+
* absolute url is prefixed with `'unsafe:'` string and only then it is written into the DOM.
229+
*
230+
* @param {RegExp=} regexp New regexp to whitelist urls with.
231+
* @returns {RegExp|ng.$compileProvider} Current RegExp if called without value or self for
232+
* chaining otherwise.
233+
*/
234+
this.urlSanitizationWhitelist = function(regexp) {
235+
if (isDefined(regexp)) {
236+
urlSanitizationWhitelist = regexp;
237+
return this;
238+
}
239+
return urlSanitizationWhitelist;
240+
};
241+
242+
212243
this.$get = [
213244
'$injector', '$interpolate', '$exceptionHandler', '$http', '$templateCache', '$parse',
214-
'$controller', '$rootScope',
245+
'$controller', '$rootScope', '$document',
215246
function($injector, $interpolate, $exceptionHandler, $http, $templateCache, $parse,
216-
$controller, $rootScope) {
247+
$controller, $rootScope, $document) {
217248

218249
var Attributes = function(element, attr) {
219250
this.$$element = element;
@@ -235,7 +266,8 @@ function $CompileProvider($provide) {
235266
*/
236267
$set: function(key, value, writeAttr, attrName) {
237268
var booleanKey = getBooleanAttrName(this.$$element[0], key),
238-
$$observers = this.$$observers;
269+
$$observers = this.$$observers,
270+
normalizedVal;
239271

240272
if (booleanKey) {
241273
this.$$element.prop(key, value);
@@ -254,6 +286,19 @@ function $CompileProvider($provide) {
254286
}
255287
}
256288

289+
290+
// sanitize a[href] values
291+
if (nodeName_(this.$$element[0]) === 'A' && key === 'href') {
292+
urlSanitizationNode.setAttribute('href', value);
293+
294+
// href property always returns normalized absolute url, so we can match against that
295+
normalizedVal = urlSanitizationNode.href;
296+
if (!normalizedVal.match(urlSanitizationWhitelist)) {
297+
this[key] = value = 'unsafe:' + normalizedVal;
298+
}
299+
}
300+
301+
257302
if (writeAttr !== false) {
258303
if (value === null || value === undefined) {
259304
this.$$element.removeAttr(attrName);
@@ -297,7 +342,8 @@ function $CompileProvider($provide) {
297342
}
298343
};
299344

300-
var startSymbol = $interpolate.startSymbol(),
345+
var urlSanitizationNode = $document[0].createElement('a'),
346+
startSymbol = $interpolate.startSymbol(),
301347
endSymbol = $interpolate.endSymbol(),
302348
denormalizeTemplate = (startSymbol == '{{' || endSymbol == '}}')
303349
? identity
@@ -748,7 +794,7 @@ function $CompileProvider($provide) {
748794
}
749795
break;
750796
}
751-
797+
752798
case '=': {
753799
parentGet = $parse(attrs[attrName]);
754800
parentSet = parentGet.assign || function() {
@@ -1029,10 +1075,10 @@ function $CompileProvider($provide) {
10291075
function addAttrInterpolateDirective(node, directives, value, name) {
10301076
var interpolateFn = $interpolate(value, true);
10311077

1032-
10331078
// no interpolation found -> ignore
10341079
if (!interpolateFn) return;
10351080

1081+
10361082
directives.push({
10371083
priority: 100,
10381084
compile: valueFn(function attrInterpolateLinkFn(scope, element, attr) {

‎src/ng/directive/booleanAttrs.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,9 @@ forEach(['src', 'href'], function(attrName) {
340340

341341
// on IE, if "ng:src" directive declaration is used and "src" attribute doesn't exist
342342
// then calling element.setAttribute('src', 'foo') doesn't do anything, so we need
343-
// to set the property as well to achieve the desired effect
344-
if (msie) element.prop(attrName, value);
343+
// to set the property as well to achieve the desired effect.
344+
// we use attr[attrName] value since $set can sanitize the url.
345+
if (msie) element.prop(attrName, attr[attrName]);
345346
});
346347
}
347348
};

‎test/ng/compileSpec.js

+141
Original file line numberDiff line numberDiff line change
@@ -2354,7 +2354,148 @@ describe('$compile', function() {
23542354
expect(jqLite(element.find('span')[1]).text()).toEqual('T:true');
23552355
});
23562356
});
2357+
});
2358+
2359+
2360+
describe('href sanitization', function() {
2361+
2362+
it('should sanitize javascript: urls', inject(function($compile, $rootScope) {
2363+
element = $compile('<a href="{{testUrl}}"></a>')($rootScope);
2364+
$rootScope.testUrl = "javascript:doEvilStuff()";
2365+
$rootScope.$apply();
2366+
2367+
expect(element.attr('href')).toBe('unsafe:javascript:doEvilStuff()');
2368+
}));
2369+
2370+
2371+
it('should sanitize data: urls', inject(function($compile, $rootScope) {
2372+
element = $compile('<a href="{{testUrl}}"></a>')($rootScope);
2373+
$rootScope.testUrl = "data:evilPayload";
2374+
$rootScope.$apply();
2375+
2376+
expect(element.attr('href')).toBe('unsafe:data:evilPayload');
2377+
}));
2378+
2379+
2380+
it('should sanitize obfuscated javascript: urls', inject(function($compile, $rootScope) {
2381+
element = $compile('<a href="{{testUrl}}"></a>')($rootScope);
2382+
2383+
// case-sensitive
2384+
$rootScope.testUrl = "JaVaScRiPt:doEvilStuff()";
2385+
$rootScope.$apply();
2386+
expect(element[0].href).toBe('unsafe:javascript:doEvilStuff()');
2387+
2388+
// tab in protocol
2389+
$rootScope.testUrl = "java\u0009script:doEvilStuff()";
2390+
$rootScope.$apply();
2391+
expect(element[0].href).toMatch(/(http:\/\/|unsafe:javascript:doEvilStuff\(\))/);
2392+
2393+
// space before
2394+
$rootScope.testUrl = " javascript:doEvilStuff()";
2395+
$rootScope.$apply();
2396+
expect(element[0].href).toBe('unsafe:javascript:doEvilStuff()');
2397+
2398+
// ws chars before
2399+
$rootScope.testUrl = " \u000e javascript:doEvilStuff()";
2400+
$rootScope.$apply();
2401+
expect(element[0].href).toMatch(/(http:\/\/|unsafe:javascript:doEvilStuff\(\))/);
2402+
2403+
// post-fixed with proper url
2404+
$rootScope.testUrl = "javascript:doEvilStuff(); http://make.me/look/good";
2405+
$rootScope.$apply();
2406+
expect(element[0].href).toBeOneOf(
2407+
'unsafe:javascript:doEvilStuff(); http://make.me/look/good',
2408+
'unsafe:javascript:doEvilStuff();%20http://make.me/look/good'
2409+
);
2410+
}));
2411+
2412+
2413+
it('should sanitize ngHref bindings as well', inject(function($compile, $rootScope) {
2414+
element = $compile('<a ng-href="{{testUrl}}"></a>')($rootScope);
2415+
$rootScope.testUrl = "javascript:doEvilStuff()";
2416+
$rootScope.$apply();
2417+
2418+
expect(element[0].href).toBe('unsafe:javascript:doEvilStuff()');
2419+
}));
2420+
2421+
2422+
it('should not sanitize valid urls', inject(function($compile, $rootScope) {
2423+
element = $compile('<a href="{{testUrl}}"></a>')($rootScope);
2424+
2425+
$rootScope.testUrl = "foo/bar";
2426+
$rootScope.$apply();
2427+
expect(element.attr('href')).toBe('foo/bar');
2428+
2429+
$rootScope.testUrl = "/foo/bar";
2430+
$rootScope.$apply();
2431+
expect(element.attr('href')).toBe('/foo/bar');
2432+
2433+
$rootScope.testUrl = "../foo/bar";
2434+
$rootScope.$apply();
2435+
expect(element.attr('href')).toBe('../foo/bar');
2436+
2437+
$rootScope.testUrl = "#foo";
2438+
$rootScope.$apply();
2439+
expect(element.attr('href')).toBe('#foo');
23572440

2441+
$rootScope.testUrl = "http://foo/bar";
2442+
$rootScope.$apply();
2443+
expect(element.attr('href')).toBe('http://foo/bar');
23582444

2445+
$rootScope.testUrl = " http://foo/bar";
2446+
$rootScope.$apply();
2447+
expect(element.attr('href')).toBe(' http://foo/bar');
2448+
2449+
$rootScope.testUrl = "https://foo/bar";
2450+
$rootScope.$apply();
2451+
expect(element.attr('href')).toBe('https://foo/bar');
2452+
2453+
$rootScope.testUrl = "ftp://foo/bar";
2454+
$rootScope.$apply();
2455+
expect(element.attr('href')).toBe('ftp://foo/bar');
2456+
2457+
$rootScope.testUrl = "mailto:[email protected]";
2458+
$rootScope.$apply();
2459+
expect(element.attr('href')).toBe('mailto:[email protected]');
2460+
}));
2461+
2462+
2463+
it('should not sanitize href on elements other than anchor', inject(function($compile, $rootScope) {
2464+
element = $compile('<div href="{{testUrl}}"></div>')($rootScope);
2465+
$rootScope.testUrl = "javascript:doEvilStuff()";
2466+
$rootScope.$apply();
2467+
2468+
expect(element.attr('href')).toBe('javascript:doEvilStuff()');
2469+
}));
2470+
2471+
2472+
it('should not sanitize attributes other than href', inject(function($compile, $rootScope) {
2473+
element = $compile('<a title="{{testUrl}}"></a>')($rootScope);
2474+
$rootScope.testUrl = "javascript:doEvilStuff()";
2475+
$rootScope.$apply();
2476+
2477+
expect(element.attr('title')).toBe('javascript:doEvilStuff()');
2478+
}));
2479+
2480+
2481+
it('should allow reconfiguration of the href whitelist', function() {
2482+
module(function($compileProvider) {
2483+
expect($compileProvider.urlSanitizationWhitelist() instanceof RegExp).toBe(true);
2484+
var returnVal = $compileProvider.urlSanitizationWhitelist(/javascript:/);
2485+
expect(returnVal).toBe($compileProvider);
2486+
});
2487+
2488+
inject(function($compile, $rootScope) {
2489+
element = $compile('<a href="{{testUrl}}"></a>')($rootScope);
2490+
2491+
$rootScope.testUrl = "javascript:doEvilStuff()";
2492+
$rootScope.$apply();
2493+
expect(element.attr('href')).toBe('javascript:doEvilStuff()');
2494+
2495+
$rootScope.testUrl = "http://recon/figured";
2496+
$rootScope.$apply();
2497+
expect(element.attr('href')).toBe('unsafe:http://recon/figured');
2498+
});
2499+
});
23592500
});
23602501
});

0 commit comments

Comments
 (0)