Skip to content

Commit 68fb70e

Browse files
rjametpetebacondarwin
authored andcommitted
fix($compile): lower the $sce context for src on video, audio, source, track
Previously, video, audio, source, and track sources were $sce.RESOURCE_URL. This is not justified as no attacks (script execution) are possible through these attributes as far as we can tell. Angular2 also uses the same categorization. This change is not breaking, and uses of $sce.trustAsResourceUrl before assigning to src or ng-src attributes will just be silently ignored. This has also been given a LGTM by @mprobst via email. Commit 4853201 on the master branch contains the same changes, but is missing this commit description. This commit does not backport the BC introduced in 04cad41. PR (angular#15039) Closes angular#14019
1 parent e8e8186 commit 68fb70e

File tree

3 files changed

+58
-7
lines changed

3 files changed

+58
-7
lines changed

src/ng/compile.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -3160,11 +3160,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
31603160
return $sce.HTML;
31613161
}
31623162
var tag = nodeName_(node);
3163+
// All tags with src attributes require a RESOURCE_URL value, except for
3164+
// img and various html5 media tags.
3165+
if (attrNormalizedName === 'src' || attrNormalizedName === 'ngSrc') {
3166+
if (['img', 'video', 'audio', 'source', 'track'].indexOf(tag) === -1) {
3167+
return $sce.RESOURCE_URL;
3168+
}
31633169
// maction[xlink:href] can source SVG. It's not limited to <maction>.
3164-
if (attrNormalizedName === "xlinkHref" ||
3165-
(tag === "form" && attrNormalizedName === "action") ||
3166-
(tag !== "img" && (attrNormalizedName === "src" ||
3167-
attrNormalizedName === "ngSrc"))) {
3170+
} else if (attrNormalizedName === 'xlinkHref' ||
3171+
(tag === 'form' && attrNormalizedName === 'action')
3172+
) {
31683173
return $sce.RESOURCE_URL;
31693174
}
31703175
}

src/ng/sce.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ function $SceDelegateProvider() {
537537
* | `$sce.HTML` | For HTML that's safe to source into the application. The {@link ng.directive:ngBindHtml ngBindHtml} directive uses this context for bindings. If an unsafe value is encountered and the {@link ngSanitize $sanitize} module is present this will sanitize the value instead of throwing an error. |
538538
* | `$sce.CSS` | For CSS that's safe to source into the application. Currently unused. Feel free to use it in your own directives. |
539539
* | `$sce.URL` | For URLs that are safe to follow as links. Currently unused (`<a href=` and `<img src=` sanitize their urls and don't constitute an SCE context. |
540-
* | `$sce.RESOURCE_URL` | For URLs that are not only safe to follow as links, but whose contents are also safe to include in your application. Examples include `ng-include`, `src` / `ngSrc` bindings for tags other than `IMG` (e.g. `IFRAME`, `OBJECT`, etc.) <br><br>Note that `$sce.RESOURCE_URL` makes a stronger statement about the URL than `$sce.URL` does and therefore contexts requiring values trusted for `$sce.RESOURCE_URL` can be used anywhere that values trusted for `$sce.URL` are required. |
540+
* | `$sce.RESOURCE_URL` | For URLs that are not only safe to follow as links, but whose contents are also safe to include in your application. Examples include `ng-include`, `src` / `ngSrc` bindings for tags other than `IMG`, `VIDEO`, `AUDIO`, `SOURCE`, and `TRACK` (e.g. `IFRAME`, `OBJECT`, etc.) <br><br>Note that `$sce.RESOURCE_URL` makes a stronger statement about the URL than `$sce.URL` does and therefore contexts requiring values trusted for `$sce.RESOURCE_URL` can be used anywhere that values trusted for `$sce.URL` are required. |
541541
* | `$sce.JS` | For JavaScript that is safe to execute in your application's context. Currently unused. Feel free to use it in your own directives. |
542542
*
543543
* ## Format of items in {@link ng.$sceDelegateProvider#resourceUrlWhitelist resourceUrlWhitelist}/{@link ng.$sceDelegateProvider#resourceUrlBlacklist Blacklist} <a name="resourceUrlPatternItem"></a>

test/ng/compileSpec.js

+48-2
Original file line numberDiff line numberDiff line change
@@ -10088,8 +10088,7 @@ describe('$compile', function() {
1008810088
});
1008910089
});
1009010090

10091-
10092-
describe('img[src] sanitization', function() {
10091+
describe('*[src] context requirement', function() {
1009310092

1009410093
it('should NOT require trusted values for img src', inject(function($rootScope, $compile, $sce) {
1009510094
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);
@@ -10102,6 +10101,53 @@ describe('$compile', function() {
1010210101
expect(element.attr('src')).toEqual('http://example.com/image2.png');
1010310102
}));
1010410103

10104+
// IE9 rejects the video / audio tag with "Error: Not implemented" and the source tag with
10105+
// "Unable to get value of the property 'childNodes': object is null or undefined"
10106+
if (!msie || msie > 9) {
10107+
they('should NOT require trusted values for $prop src', ['video', 'audio'],
10108+
function(tag) {
10109+
inject(function($rootScope, $compile, $sce) {
10110+
element = $compile('<' + tag + ' src="{{testUrl}}"></' + tag + '>')($rootScope);
10111+
$rootScope.testUrl = 'http://example.com/image.mp4';
10112+
$rootScope.$digest();
10113+
expect(element.attr('src')).toEqual('http://example.com/image.mp4');
10114+
10115+
// But it should accept trusted values anyway.
10116+
$rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.mp4');
10117+
$rootScope.$digest();
10118+
expect(element.attr('src')).toEqual('http://example.com/image2.mp4');
10119+
10120+
// and trustedResourceUrls for retrocompatibility
10121+
$rootScope.testUrl = $sce.trustAsResourceUrl('http://example.com/image3.mp4');
10122+
$rootScope.$digest();
10123+
expect(element.attr('src')).toEqual('http://example.com/image3.mp4');
10124+
});
10125+
});
10126+
10127+
they('should NOT require trusted values for $prop src', ['source', 'track'],
10128+
function(tag) {
10129+
inject(function($rootScope, $compile, $sce) {
10130+
element = $compile('<video><' + tag + ' src="{{testUrl}}"></' + tag + '></video>')($rootScope);
10131+
$rootScope.testUrl = 'http://example.com/image.mp4';
10132+
$rootScope.$digest();
10133+
expect(element.find(tag).attr('src')).toEqual('http://example.com/image.mp4');
10134+
10135+
// But it should accept trusted values anyway.
10136+
$rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.mp4');
10137+
$rootScope.$digest();
10138+
expect(element.find(tag).attr('src')).toEqual('http://example.com/image2.mp4');
10139+
10140+
// and trustedResourceUrls for retrocompatibility
10141+
$rootScope.testUrl = $sce.trustAsResourceUrl('http://example.com/image3.mp4');
10142+
$rootScope.$digest();
10143+
expect(element.find(tag).attr('src')).toEqual('http://example.com/image3.mp4');
10144+
});
10145+
});
10146+
}
10147+
});
10148+
10149+
describe('img[src] sanitization', function() {
10150+
1010510151
it('should not sanitize attributes other than src', inject(function($compile, $rootScope) {
1010610152
element = $compile('<img title="{{testUrl}}"></img>')($rootScope);
1010710153
$rootScope.testUrl = "javascript:doEvilStuff()";

0 commit comments

Comments
 (0)