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

Commit 36c4de9

Browse files
feat($sce): handle URLs through the $sce service
Thanks to @rjamet for the original work on this feature. This is a large patch to handle URLs with the $sce service, similarly to HTML context. However, to keep rough compatibility with existing apps, we need to allow URL-context concatenation, since previously $sce contexts prevented sanitization. There's now a set of special contexts (defined in $interpolate) that allow concatenation, in a roughly intuitive way: * Trusted types alone are trusted, e.g. `"{{safeType}}"` will not be sanitized. * Any concatenation of values results in a non-trusted type that will be handled by `getTrusted` once the concatenation is done, e.g. `"{{ 'javascript:foo' }}"` and `"javascript:{{safeType}}"` will be sanitized. This commit also introduces a new SCE context called `SRC`, which represents a URL being used as a source for an image, video, audio, etc. The hierarchy is setup so that the `URL` context is also a `SRC` context, in the same way that the `RESOURCE_URL` context is also a `URL` (and now also a `SRC`) context. Where we previously sanitized URL attributes inside the compiler, we now only apply the `$sce.URL` or `$sce.SRC` context requirement. * When calling `getTrustedSrc()` a value that is not already a trusted `SRC` will be sanitized using the `imgSrcSanitizationWhitelist` * When calling `getTrustedUrl()` a value that is not already a trusted `URL` will be sanitized using the `aHrefSanitizationWhitelist` This results in behaviour that closely matches the previous sanitization behaviour. BREAKING CHANGES: If you use `attrs.$set` for URL attributes there will be no automated sanitization of the URL value. This is now in line with other contexts. If you are programmatically writing URL values to attributes from untrusted input then you must sanitize it yourself (possibly by calling the private `$$sanitizeUri` service).
1 parent 7df2952 commit 36c4de9

File tree

11 files changed

+542
-248
lines changed

11 files changed

+542
-248
lines changed

src/ng/compile.js

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,9 +1528,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
15281528

15291529
this.$get = [
15301530
'$injector', '$interpolate', '$exceptionHandler', '$templateRequest', '$parse',
1531-
'$controller', '$rootScope', '$sce', '$animate', '$$sanitizeUri',
1531+
'$controller', '$rootScope', '$sce', '$animate',
15321532
function($injector, $interpolate, $exceptionHandler, $templateRequest, $parse,
1533-
$controller, $rootScope, $sce, $animate, $$sanitizeUri) {
1533+
$controller, $rootScope, $sce, $animate) {
15341534

15351535
var SIMPLE_ATTR_NAME = /^\w/;
15361536
var specialAttrHolder = window.document.createElement('div');
@@ -1710,12 +1710,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
17101710

17111711
nodeName = nodeName_(this.$$element);
17121712

1713-
if ((nodeName === 'a' && (key === 'href' || key === 'xlinkHref')) ||
1714-
(nodeName === 'img' && key === 'src') ||
1715-
(nodeName === 'image' && key === 'xlinkHref')) {
1716-
// sanitize a[href] and img[src] values
1717-
this[key] = value = $$sanitizeUri(value, nodeName === 'img' || nodeName === 'image');
1718-
} else if (nodeName === 'img' && key === 'srcset' && isDefined(value)) {
1713+
// img[srcset] is a bit too weird of a beast to handle through $sce, for now at least.
1714+
// Instead, we sanitize each of the URIs individually, which works, even dynamically.
1715+
// It's not possible to work around this using `$sce.trustAsUrl`.
1716+
// Instead, if you want several unsafe URLs as-is, you should probably
1717+
// use `$sce.trustAsHtml` on the whole `img` tag.
1718+
if (nodeName === 'img' && key === 'srcset' && value) {
1719+
17191720
// sanitize img[srcset] values
17201721
var result = '';
17211722

@@ -1733,16 +1734,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
17331734
for (var i = 0; i < nbrUrisWith2parts; i++) {
17341735
var innerIdx = i * 2;
17351736
// sanitize the uri
1736-
result += $$sanitizeUri(trim(rawUris[innerIdx]), true);
1737+
result += $sce.getTrustedUrl(trim(rawUris[innerIdx]));
17371738
// add the descriptor
1738-
result += (' ' + trim(rawUris[innerIdx + 1]));
1739+
result += ' ' + trim(rawUris[innerIdx + 1]);
17391740
}
17401741

17411742
// split the last item into uri and descriptor
17421743
var lastTuple = trim(rawUris[i * 2]).split(/\s/);
17431744

17441745
// sanitize the last uri
1745-
result += $$sanitizeUri(trim(lastTuple[0]), true);
1746+
result += $sce.getTrustedUrl(trim(lastTuple[0]));
17461747

17471748
// and add the last descriptor if any
17481749
if (lastTuple.length === 2) {
@@ -3268,14 +3269,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
32683269
}
32693270
var tag = nodeName_(node);
32703271
// All tags with src attributes require a RESOURCE_URL value, except for
3271-
// img and various html5 media tags.
3272+
// img and various html5 media tags, which require the SRC context.
32723273
if (attrNormalizedName === 'src' || attrNormalizedName === 'ngSrc') {
32733274
if (['img', 'video', 'audio', 'source', 'track'].indexOf(tag) === -1) {
32743275
return $sce.RESOURCE_URL;
32753276
}
3277+
return $sce.SRC;
3278+
} else if (attrNormalizedName === 'xlinkHref') {
3279+
// Some xlink:href are okay, most aren't
3280+
if (tag === 'image') return $sce.SRC;
3281+
if (tag === 'a') return $sce.URL;
3282+
return $sce.RESOURCE_URL;
32763283
} else if (
3277-
// Some xlink:href are okay, most aren't
3278-
(attrNormalizedName === 'xlinkHref' && (tag !== 'image' && tag !== 'a')) ||
32793284
// Formaction
32803285
(tag === 'form' && attrNormalizedName === 'action') ||
32813286
// If relative URLs can go where they are not expected to, then
@@ -3285,6 +3290,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
32853290
(tag === 'link' && attrNormalizedName === 'href')
32863291
) {
32873292
return $sce.RESOURCE_URL;
3293+
} else if (tag === 'a' && (attrNormalizedName === 'href' ||
3294+
attrNormalizedName === 'ngHref')) {
3295+
return $sce.URL;
32883296
}
32893297
}
32903298

src/ng/directive/attrs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ forEach(['src', 'srcset', 'href'], function(attrName) {
436436
// On IE, if "ng:src" directive declaration is used and "src" attribute doesn't exist
437437
// then calling element.setAttribute('src', 'foo') doesn't do anything, so we need
438438
// to set the property as well to achieve the desired effect.
439-
// We use attr[attrName] value since $set can sanitize the url.
439+
// We use attr[attrName] value since $set might have sanitized the url.
440440
if (msie && propName) element.prop(propName, attr[name]);
441441
});
442442
}

src/ng/interpolate.js

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,10 @@ function $InterpolateProvider() {
119119
return unwatch;
120120
}
121121

122+
function isConcatenationAllowed(context) {
123+
return context === $sce.URL || context === $sce.SRC;
124+
}
125+
122126
/**
123127
* @ngdoc service
124128
* @name $interpolate
@@ -260,7 +264,9 @@ function $InterpolateProvider() {
260264
textLength = text.length,
261265
exp,
262266
concat = [],
263-
expressionPositions = [];
267+
expressionPositions = [],
268+
singleExpression = false,
269+
contextAllowsConcatenation = isConcatenationAllowed(trustedContext);
264270

265271
while (index < textLength) {
266272
if (((startIndex = text.indexOf(startSymbol, index)) !== -1) &&
@@ -273,7 +279,7 @@ function $InterpolateProvider() {
273279
parseFns.push($parse(exp, parseStringifyInterceptor));
274280
index = endIndex + endSymbolLength;
275281
expressionPositions.push(concat.length);
276-
concat.push('');
282+
concat.push(''); // Placeholder that will get replaced with the evaluated expression.
277283
} else {
278284
// we did not find an interpolation, so we have to add the remainder to the separators array
279285
if (index !== textLength) {
@@ -283,27 +289,55 @@ function $InterpolateProvider() {
283289
}
284290
}
285291

292+
if (concat.length === 1 && expressionPositions.length === 1) {
293+
singleExpression = true;
294+
}
295+
286296
// Concatenating expressions makes it hard to reason about whether some combination of
287297
// concatenated values are unsafe to use and could easily lead to XSS. By requiring that a
288-
// single expression be used for iframe[src], object[src], etc., we ensure that the value
289-
// that's used is assigned or constructed by some JS code somewhere that is more testable or
290-
// make it obvious that you bound the value to some user controlled value. This helps reduce
291-
// the load when auditing for XSS issues.
292-
if (trustedContext && concat.length > 1) {
293-
$interpolateMinErr.throwNoconcat(text);
294-
}
298+
// single expression be used for some $sce-managed secure contexts (RESOURCE_URLs mostly),
299+
// we ensure that the value that's used is assigned or constructed by some JS code somewhere
300+
// that is more testable or make it obvious that you bound the value to some user controlled
301+
// value. This helps reduce the load when auditing for XSS issues.
302+
303+
// Note that URL and SRC $sce contexts do not need this, since `$sce` can sanitize the values
304+
// passed to it. In that case, `$sce.getTrusted` will be called on either the single expression
305+
// or on the overall concatenated string (losing trusted types used in the mix, by design).
306+
// Both these methods will sanitize plain strings. Also, HTML could be included, but since it's
307+
// only used in srcdoc attributes, this would not be very useful.
295308

296309
if (!mustHaveExpression || expressions.length) {
297310
var compute = function(values) {
298311
for (var i = 0, ii = expressions.length; i < ii; i++) {
299312
if (allOrNothing && isUndefined(values[i])) return;
300313
concat[expressionPositions[i]] = values[i];
301314
}
302-
return concat.join('');
315+
316+
if (contextAllowsConcatenation) {
317+
if (singleExpression) {
318+
// The raw value was left as-is by parseStringifyInterceptor
319+
return $sce.getTrusted(trustedContext, concat[0]);
320+
} else {
321+
return $sce.getTrusted(trustedContext, concat.join(''));
322+
}
323+
} else if (trustedContext) {
324+
if (concat.length > 1) {
325+
// there's at least two parts, so expr + string or exp + exp, and this context
326+
// doesn't allow that.
327+
$interpolateMinErr.throwNoconcat(text);
328+
} else {
329+
return concat.join('');
330+
}
331+
} else { // In an unprivileged context, just concatenate and return.
332+
return concat.join('');
333+
}
303334
};
304335

305336
var getValue = function(value) {
306-
return trustedContext ?
337+
// In concatenable contexts, getTrusted comes at the end, to avoid sanitizing individual
338+
// parts of a full URL. We don't care about losing the trustedness here, that's handled in
339+
// parseStringifyInterceptor below.
340+
return (trustedContext && !contextAllowsConcatenation) ?
307341
$sce.getTrusted(trustedContext, value) :
308342
$sce.valueOf(value);
309343
};
@@ -340,8 +374,13 @@ function $InterpolateProvider() {
340374

341375
function parseStringifyInterceptor(value) {
342376
try {
343-
value = getValue(value);
344-
return allOrNothing && !isDefined(value) ? value : stringify(value);
377+
if (contextAllowsConcatenation && singleExpression) {
378+
// No stringification in this case, to keep the trusted value until unwrapping.
379+
return value;
380+
} else {
381+
value = getValue(value);
382+
return allOrNothing && !isDefined(value) ? value : stringify(value);
383+
}
345384
} catch (err) {
346385
$exceptionHandler($interpolateMinErr.interr(text, err));
347386
}

src/ng/sanitizeUri.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* Private service to sanitize uris for links and images. Used by $compile and $sanitize.
77
*/
88
function $$SanitizeUriProvider() {
9+
910
var aHrefSanitizationWhitelist = /^\s*(https?|s?ftp|mailto|tel|file):/,
1011
imgSrcSanitizationWhitelist = /^\s*((https?|ftp|file|blob):|data:image\/)/;
1112

@@ -19,7 +20,8 @@ function $$SanitizeUriProvider() {
1920
* Any url about to be assigned to a[href] via data-binding is first normalized and turned into
2021
* an absolute url. Afterwards, the url is matched against the `aHrefSanitizationWhitelist`
2122
* regular expression. If a match is found, the original url is written into the dom. Otherwise,
22-
* the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM.
23+
* the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM,
24+
* making it inactive.
2325
*
2426
* @param {RegExp=} regexp New regexp to whitelist urls with.
2527
* @returns {RegExp|ng.$compileProvider} Current RegExp if called without value or self for
@@ -44,7 +46,8 @@ function $$SanitizeUriProvider() {
4446
* Any url about to be assigned to img[src] via data-binding is first normalized and turned into
4547
* an absolute url. Afterwards, the url is matched against the `imgSrcSanitizationWhitelist`
4648
* regular expression. If a match is found, the original url is written into the dom. Otherwise,
47-
* the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM.
49+
* the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM,
50+
* making it inactive.
4851
*
4952
* @param {RegExp=} regexp New regexp to whitelist urls with.
5053
* @returns {RegExp|ng.$compileProvider} Current RegExp if called without value or self for

0 commit comments

Comments
 (0)