Skip to content

Commit 4853201

Browse files
authored
fix($compile): lower the $sce context for src on video, audio, source, and 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. PR (angular#15039) Closes angular#14019
1 parent 13c2522 commit 4853201

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
@@ -3180,13 +3180,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
31803180
return $sce.HTML;
31813181
}
31823182
var tag = nodeName_(node);
3183+
// All tags with src attributes require a RESOURCE_URL value, except for
3184+
// img and various html5 media tags.
3185+
if (attrNormalizedName === 'src' || attrNormalizedName === 'ngSrc') {
3186+
if (['img', 'video', 'audio', 'source', 'track'].indexOf(tag) === -1) {
3187+
return $sce.RESOURCE_URL;
3188+
}
31833189
// maction[xlink:href] can source SVG. It's not limited to <maction>.
3184-
if (attrNormalizedName === 'xlinkHref' ||
3190+
} else if (attrNormalizedName === 'xlinkHref' ||
31853191
(tag === 'form' && attrNormalizedName === 'action') ||
31863192
// links can be stylesheets or imports, which can run script in the current origin
3187-
(tag === 'link' && attrNormalizedName === 'href') ||
3188-
(tag !== 'img' && (attrNormalizedName === 'src' ||
3189-
attrNormalizedName === 'ngSrc'))) {
3193+
(tag === 'link' && attrNormalizedName === 'href')
3194+
) {
31903195
return $sce.RESOURCE_URL;
31913196
}
31923197
}

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` and `SOURCE` (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
@@ -10219,8 +10219,7 @@ describe('$compile', function() {
1021910219
);
1022010220
});
1022110221

10222-
10223-
describe('img[src] sanitization', function() {
10222+
describe('*[src] context requirement', function() {
1022410223

1022510224
it('should NOT require trusted values for img src', inject(function($rootScope, $compile, $sce) {
1022610225
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);
@@ -10233,6 +10232,53 @@ describe('$compile', function() {
1023310232
expect(element.attr('src')).toEqual('http://example.com/image2.png');
1023410233
}));
1023510234

10235+
// IE9 rejects the video / audio tag with "Error: Not implemented" and the source tag with
10236+
// "Unable to get value of the property 'childNodes': object is null or undefined"
10237+
if (!msie || msie > 9) {
10238+
they('should NOT require trusted values for $prop src', ['video', 'audio'],
10239+
function(tag) {
10240+
inject(function($rootScope, $compile, $sce) {
10241+
element = $compile('<' + tag + ' src="{{testUrl}}"></' + tag + '>')($rootScope);
10242+
$rootScope.testUrl = 'http://example.com/image.mp4';
10243+
$rootScope.$digest();
10244+
expect(element.attr('src')).toEqual('http://example.com/image.mp4');
10245+
10246+
// But it should accept trusted values anyway.
10247+
$rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.mp4');
10248+
$rootScope.$digest();
10249+
expect(element.attr('src')).toEqual('http://example.com/image2.mp4');
10250+
10251+
// and trustedResourceUrls for retrocompatibility
10252+
$rootScope.testUrl = $sce.trustAsResourceUrl('http://example.com/image3.mp4');
10253+
$rootScope.$digest();
10254+
expect(element.attr('src')).toEqual('http://example.com/image3.mp4');
10255+
});
10256+
});
10257+
10258+
they('should NOT require trusted values for $prop src', ['source', 'track'],
10259+
function(tag) {
10260+
inject(function($rootScope, $compile, $sce) {
10261+
element = $compile('<video><' + tag + ' src="{{testUrl}}"></' + tag + '></video>')($rootScope);
10262+
$rootScope.testUrl = 'http://example.com/image.mp4';
10263+
$rootScope.$digest();
10264+
expect(element.find(tag).attr('src')).toEqual('http://example.com/image.mp4');
10265+
10266+
// But it should accept trusted values anyway.
10267+
$rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.mp4');
10268+
$rootScope.$digest();
10269+
expect(element.find(tag).attr('src')).toEqual('http://example.com/image2.mp4');
10270+
10271+
// and trustedResourceUrls for retrocompatibility
10272+
$rootScope.testUrl = $sce.trustAsResourceUrl('http://example.com/image3.mp4');
10273+
$rootScope.$digest();
10274+
expect(element.find(tag).attr('src')).toEqual('http://example.com/image3.mp4');
10275+
});
10276+
});
10277+
}
10278+
});
10279+
10280+
describe('img[src] sanitization', function() {
10281+
1023610282
it('should not sanitize attributes other than src', inject(function($compile, $rootScope) {
1023710283
element = $compile('<img title="{{testUrl}}"></img>')($rootScope);
1023810284
$rootScope.testUrl = 'javascript:doEvilStuff()';

0 commit comments

Comments
 (0)