Skip to content

Commit 3335234

Browse files
tboschtbosch
committed
fix($sanitize): Use same whitelist mechanism as $compile does.
`$sanitize` now uses the same mechanism as `$compile` to validate uris. By this, the validation in `$sanitize` is more general and can be configured in the same way as the one in `$compile`. Changes - Creates the new private service `$$sanitizeUri`. - Moves related specs from `compileSpec.js` into `sanitizeUriSpec.js`. - Refactors the `linky` filter to be less dependent on `$sanitize` internal functions. Fixes angular#3748.
1 parent 68ceb17 commit 3335234

9 files changed

Lines changed: 550 additions & 339 deletions

File tree

‎angularFiles.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ angularFiles = {
2727
'src/ng/parse.js',
2828
'src/ng/q.js',
2929
'src/ng/rootScope.js',
30+
'src/ng/sanitizeUri.js',
3031
'src/ng/sce.js',
3132
'src/ng/sniffer.js',
3233
'src/ng/timeout.js',

‎src/AngularPublic.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
$ParseProvider,
6666
$RootScopeProvider,
6767
$QProvider,
68+
$$SanitizeUriProvider,
6869
$SceProvider,
6970
$SceDelegateProvider,
7071
$SnifferProvider,
@@ -136,6 +137,10 @@ function publishExternalAPI(angular){
136137

137138
angularModule('ng', ['ngLocale'], ['$provide',
138139
function ngModule($provide) {
140+
// $$sanitizeUriProvider needs to be before $compileProvider as it is used by it.
141+
$provide.provider({
142+
$$sanitizeUri: $$SanitizeUriProvider
143+
});
139144
$provide.provider('$compile', $CompileProvider).
140145
directive({
141146
a: htmlAnchorDirective,

‎src/ng/compile.js‎

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -493,14 +493,12 @@ var $compileMinErr = minErr('$compile');
493493
*
494494
* @description
495495
*/
496-
$CompileProvider.$inject = ['$provide'];
497-
function $CompileProvider($provide) {
496+
$CompileProvider.$inject = ['$provide', '$$sanitizeUriProvider'];
497+
function $CompileProvider($provide, $$sanitizeUriProvider) {
498498
var hasDirectives = {},
499499
Suffix = 'Directive',
500500
COMMENT_DIRECTIVE_REGEXP = /^\s*directive\:\s*([\d\w\-_]+)\s+(.*)$/,
501-
CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/,
502-
aHrefSanitizationWhitelist = /^\s*(https?|ftp|mailto|tel|file):/,
503-
imgSrcSanitizationWhitelist = /^\s*(https?|ftp|file):|data:image\//;
501+
CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/;
504502

505503
// Ref: http://developers.whatwg.org/webappapis.html#event-handler-idl-attributes
506504
// The assumption is that future DOM event attribute names will begin with
@@ -584,10 +582,11 @@ function $CompileProvider($provide) {
584582
*/
585583
this.aHrefSanitizationWhitelist = function(regexp) {
586584
if (isDefined(regexp)) {
587-
aHrefSanitizationWhitelist = regexp;
585+
$$sanitizeUriProvider.aHrefSanitizationWhitelist(regexp);
588586
return this;
587+
} else {
588+
return $$sanitizeUriProvider.aHrefSanitizationWhitelist();
589589
}
590-
return aHrefSanitizationWhitelist;
591590
};
592591

593592

@@ -614,18 +613,18 @@ function $CompileProvider($provide) {
614613
*/
615614
this.imgSrcSanitizationWhitelist = function(regexp) {
616615
if (isDefined(regexp)) {
617-
imgSrcSanitizationWhitelist = regexp;
616+
$$sanitizeUriProvider.imgSrcSanitizationWhitelist(regexp);
618617
return this;
618+
} else {
619+
return $$sanitizeUriProvider.imgSrcSanitizationWhitelist();
619620
}
620-
return imgSrcSanitizationWhitelist;
621621
};
622622

623-
624623
this.$get = [
625624
'$injector', '$interpolate', '$exceptionHandler', '$http', '$templateCache', '$parse',
626-
'$controller', '$rootScope', '$document', '$sce', '$animate',
625+
'$controller', '$rootScope', '$document', '$sce', '$animate', '$$sanitizeUri',
627626
function($injector, $interpolate, $exceptionHandler, $http, $templateCache, $parse,
628-
$controller, $rootScope, $document, $sce, $animate) {
627+
$controller, $rootScope, $document, $sce, $animate, $$sanitizeUri) {
629628

630629
var Attributes = function(element, attr) {
631630
this.$$element = element;
@@ -730,16 +729,7 @@ function $CompileProvider($provide) {
730729
// sanitize a[href] and img[src] values
731730
if ((nodeName === 'A' && key === 'href') ||
732731
(nodeName === 'IMG' && key === 'src')) {
733-
// NOTE: urlResolve() doesn't support IE < 8 so we don't sanitize for that case.
734-
if (!msie || msie >= 8 ) {
735-
normalizedVal = urlResolve(value).href;
736-
if (normalizedVal !== '') {
737-
if ((key === 'href' && !normalizedVal.match(aHrefSanitizationWhitelist)) ||
738-
(key === 'src' && !normalizedVal.match(imgSrcSanitizationWhitelist))) {
739-
this[key] = value = 'unsafe:' + normalizedVal;
740-
}
741-
}
742-
}
732+
this[key] = value = $$sanitizeUri(value, key === 'src');
743733
}
744734

745735
if (writeAttr !== false) {

‎src/ng/sanitizeUri.js‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
'use strict';
2+
3+
/**
4+
* @description
5+
* Private service to sanitize uris for links and images. Used by $compile and $sanitize.
6+
*/
7+
function $$SanitizeUriProvider() {
8+
var aHrefSanitizationWhitelist = /^\s*(https?|ftp|mailto|tel|file):/,
9+
imgSrcSanitizationWhitelist = /^\s*(https?|ftp|file):|data:image\//;
10+
11+
/**
12+
* @description
13+
* Retrieves or overrides the default regular expression that is used for whitelisting of safe
14+
* urls during a[href] sanitization.
15+
*
16+
* The sanitization is a security measure aimed at prevent XSS attacks via html links.
17+
*
18+
* Any url about to be assigned to a[href] via data-binding is first normalized and turned into
19+
* an absolute url. Afterwards, the url is matched against the `aHrefSanitizationWhitelist`
20+
* regular expression. If a match is found, the original url is written into the dom. Otherwise,
21+
* the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM.
22+
*
23+
* @param {RegExp=} regexp New regexp to whitelist urls with.
24+
* @returns {RegExp|ng.$compileProvider} Current RegExp if called without value or self for
25+
* chaining otherwise.
26+
*/
27+
this.aHrefSanitizationWhitelist = function(regexp) {
28+
if (isDefined(regexp)) {
29+
aHrefSanitizationWhitelist = regexp;
30+
return this;
31+
}
32+
return aHrefSanitizationWhitelist;
33+
};
34+
35+
36+
/**
37+
* @description
38+
* Retrieves or overrides the default regular expression that is used for whitelisting of safe
39+
* urls during img[src] sanitization.
40+
*
41+
* The sanitization is a security measure aimed at prevent XSS attacks via html links.
42+
*
43+
* Any url about to be assigned to img[src] via data-binding is first normalized and turned into
44+
* an absolute url. Afterwards, the url is matched against the `imgSrcSanitizationWhitelist`
45+
* regular expression. If a match is found, the original url is written into the dom. Otherwise,
46+
* the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM.
47+
*
48+
* @param {RegExp=} regexp New regexp to whitelist urls with.
49+
* @returns {RegExp|ng.$compileProvider} Current RegExp if called without value or self for
50+
* chaining otherwise.
51+
*/
52+
this.imgSrcSanitizationWhitelist = function(regexp) {
53+
if (isDefined(regexp)) {
54+
imgSrcSanitizationWhitelist = regexp;
55+
return this;
56+
}
57+
return imgSrcSanitizationWhitelist;
58+
};
59+
60+
this.$get = function() {
61+
return function sanitizeUri(uri, isImage) {
62+
var regex = isImage ? imgSrcSanitizationWhitelist : aHrefSanitizationWhitelist;
63+
var normalizedVal;
64+
// NOTE: urlResolve() doesn't support IE < 8 so we don't sanitize for that case.
65+
if (!msie || msie >= 8 ) {
66+
normalizedVal = urlResolve(uri).href;
67+
if (normalizedVal !== '' && !normalizedVal.match(regex)) {
68+
return 'unsafe:'+normalizedVal;
69+
}
70+
}
71+
return uri;
72+
};
73+
};
74+
}

‎src/ngSanitize/filter/linky.js‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
/* global htmlSanitizeWriter: false */
3+
/* global sanitizeText: false */
44

55
/**
66
* @ngdoc filter
@@ -100,7 +100,7 @@
100100
</doc:scenario>
101101
</doc:example>
102102
*/
103-
angular.module('ngSanitize').filter('linky', function() {
103+
angular.module('ngSanitize').filter('linky', ['$sanitize', function($sanitize) {
104104
var LINKY_URL_REGEXP =
105105
/((ftp|https?):\/\/|(mailto:)?[A-Za-z0-9._%+-]+@)\S*[^\s.;,(){}<>]/,
106106
MAILTO_REGEXP = /^mailto:/;
@@ -110,28 +110,40 @@ angular.module('ngSanitize').filter('linky', function() {
110110
var match;
111111
var raw = text;
112112
var html = [];
113-
// TODO(vojta): use $sanitize instead
114-
var writer = htmlSanitizeWriter(html);
115113
var url;
116114
var i;
117-
var properties = {};
118-
if (angular.isDefined(target)) {
119-
properties.target = target;
120-
}
121115
while ((match = raw.match(LINKY_URL_REGEXP))) {
122116
// We can not end in these as they are sometimes found at the end of the sentence
123117
url = match[0];
124118
// if we did not match ftp/http/mailto then assume mailto
125119
if (match[2] == match[3]) url = 'mailto:' + url;
126120
i = match.index;
127-
writer.chars(raw.substr(0, i));
128-
properties.href = url;
129-
writer.start('a', properties);
130-
writer.chars(match[0].replace(MAILTO_REGEXP, ''));
131-
writer.end('a');
121+
addText(raw.substr(0, i));
122+
addLink(url, match[0].replace(MAILTO_REGEXP, ''));
132123
raw = raw.substring(i + match[0].length);
133124
}
134-
writer.chars(raw);
135-
return html.join('');
125+
addText(raw);
126+
return $sanitize(html.join(''));
127+
128+
function addText(text) {
129+
if (!text) {
130+
return;
131+
}
132+
html.push(sanitizeText(text));
133+
}
134+
135+
function addLink(url, text) {
136+
html.push('<a ');
137+
if (angular.isDefined(target)) {
138+
html.push('target="');
139+
html.push(target);
140+
html.push('" ');
141+
}
142+
html.push('href="');
143+
html.push(url);
144+
html.push('">');
145+
addText(text);
146+
html.push('</a>');
147+
}
136148
};
137-
});
149+
}]);

‎src/ngSanitize/sanitize.js‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ var $sanitizeMinErr = angular.$$minErr('$sanitize');
4646
* it into the returned string, however, since our parser is more strict than a typical browser
4747
* parser, it's possible that some obscure input, which would be recognized as valid HTML by a
4848
* browser, won't make it through the sanitizer.
49+
* The whitelist is configured using the functions `aHrefSanitizationWhitelist` and
50+
* `imgSrcSanitizationWhitelist` of {@link ng.$compileProvider `$compileProvider`}.
4951
*
5052
* @param {string} html Html input.
5153
* @returns {string} Sanitized html.
@@ -128,11 +130,24 @@ var $sanitizeMinErr = angular.$$minErr('$sanitize');
128130
</doc:scenario>
129131
</doc:example>
130132
*/
131-
var $sanitize = function(html) {
133+
function $SanitizeProvider() {
134+
this.$get = ['$$sanitizeUri', function($$sanitizeUri) {
135+
return function(html) {
136+
var buf = [];
137+
htmlParser(html, htmlSanitizeWriter(buf, function(uri, isImage) {
138+
return !/^unsafe/.test($$sanitizeUri(uri, isImage));
139+
}));
140+
return buf.join('');
141+
};
142+
}];
143+
}
144+
145+
function sanitizeText(chars) {
132146
var buf = [];
133-
htmlParser(html, htmlSanitizeWriter(buf));
134-
return buf.join('');
135-
};
147+
var writer = htmlSanitizeWriter(buf, angular.noop);
148+
writer.chars(chars);
149+
return buf.join('');
150+
}
136151

137152

138153
// Regular Expressions for parsing tags and attributes
@@ -145,7 +160,6 @@ var START_TAG_REGEXP =
145160
COMMENT_REGEXP = /<!--(.*?)-->/g,
146161
DOCTYPE_REGEXP = /<!DOCTYPE([^>]*?)>/i,
147162
CDATA_REGEXP = /<!\[CDATA\[(.*?)]]>/g,
148-
URI_REGEXP = /^((ftp|https?):\/\/|mailto:|tel:|#)/i,
149163
// Match everything outside of normal chars and " (quote character)
150164
NON_ALPHANUMERIC_REGEXP = /([^\#-~| |!])/g;
151165

@@ -353,8 +367,18 @@ function htmlParser( html, handler ) {
353367
*/
354368
var hiddenPre=document.createElement("pre");
355369
function decodeEntities(value) {
356-
hiddenPre.innerHTML=value.replace(/</g,"&lt;");
357-
return hiddenPre.innerText || hiddenPre.textContent || '';
370+
if (!value) {
371+
return '';
372+
}
373+
// Note: IE8 does not preserve spaces at the start/end of innerHTML
374+
var spaceRe = /^(\s*)([\s\S]*?)(\s*)$/;
375+
var parts = spaceRe.exec(value);
376+
parts[0] = '';
377+
if (parts[2]) {
378+
hiddenPre.innerHTML=parts[2].replace(/</g,"&lt;");
379+
parts[2] = hiddenPre.innerText || hiddenPre.textContent;
380+
}
381+
return parts.join('');
358382
}
359383

360384
/**
@@ -384,7 +408,7 @@ function encodeEntities(value) {
384408
* comment: function(text) {}
385409
* }
386410
*/
387-
function htmlSanitizeWriter(buf){
411+
function htmlSanitizeWriter(buf, uriValidator){
388412
var ignore = false;
389413
var out = angular.bind(buf, buf.push);
390414
return {
@@ -398,7 +422,9 @@ function htmlSanitizeWriter(buf){
398422
out(tag);
399423
angular.forEach(attrs, function(value, key){
400424
var lkey=angular.lowercase(key);
401-
if (validAttrs[lkey]===true && (uriAttrs[lkey]!==true || value.match(URI_REGEXP))) {
425+
var isImage = (tag === 'img' && lkey === 'src') || (lkey === 'background');
426+
if (validAttrs[lkey] === true &&
427+
(uriAttrs[lkey] !== true || uriValidator(value, isImage))) {
402428
out(' ');
403429
out(key);
404430
out('="');
@@ -430,4 +456,4 @@ function htmlSanitizeWriter(buf){
430456

431457

432458
// define ngSanitize module and register $sanitize service
433-
angular.module('ngSanitize', []).value('$sanitize', $sanitize);
459+
angular.module('ngSanitize', []).provider('$sanitize', $SanitizeProvider);

0 commit comments

Comments
 (0)