From 485320129dd8a942acfcb1e9388eb09667f383b6 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Fri, 26 Aug 2016 09:52:11 +0200 Subject: [PATCH] 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 (#15039) Closes #14019 --- src/ng/compile.js | 13 +++++++---- src/ng/sce.js | 2 +- test/ng/compileSpec.js | 50 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index dc2ecd456..37e690e49 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3180,13 +3180,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return $sce.HTML; } var tag = nodeName_(node); + // All tags with src attributes require a RESOURCE_URL value, except for + // img and various html5 media tags. + if (attrNormalizedName === 'src' || attrNormalizedName === 'ngSrc') { + if (['img', 'video', 'audio', 'source', 'track'].indexOf(tag) === -1) { + return $sce.RESOURCE_URL; + } // maction[xlink:href] can source SVG. It's not limited to . - if (attrNormalizedName === 'xlinkHref' || + } else if (attrNormalizedName === 'xlinkHref' || (tag === 'form' && attrNormalizedName === 'action') || // links can be stylesheets or imports, which can run script in the current origin - (tag === 'link' && attrNormalizedName === 'href') || - (tag !== 'img' && (attrNormalizedName === 'src' || - attrNormalizedName === 'ngSrc'))) { + (tag === 'link' && attrNormalizedName === 'href') + ) { return $sce.RESOURCE_URL; } } diff --git a/src/ng/sce.js b/src/ng/sce.js index f16aee559..a530f2f37 100644 --- a/src/ng/sce.js +++ b/src/ng/sce.js @@ -537,7 +537,7 @@ function $SceDelegateProvider() { * | `$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. | * | `$sce.CSS` | For CSS that's safe to source into the application. Currently unused. Feel free to use it in your own directives. | * | `$sce.URL` | For URLs that are safe to follow as links. Currently unused (`
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. | + * | `$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.)

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. | * | `$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. | * * ## Format of items in {@link ng.$sceDelegateProvider#resourceUrlWhitelist resourceUrlWhitelist}/{@link ng.$sceDelegateProvider#resourceUrlBlacklist Blacklist} diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 2d3ed295a..0ccccb713 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -10219,8 +10219,7 @@ describe('$compile', function() { ); }); - - describe('img[src] sanitization', function() { + describe('*[src] context requirement', function() { it('should NOT require trusted values for img src', inject(function($rootScope, $compile, $sce) { element = $compile('')($rootScope); @@ -10233,6 +10232,53 @@ describe('$compile', function() { expect(element.attr('src')).toEqual('http://example.com/image2.png'); })); + // IE9 rejects the video / audio tag with "Error: Not implemented" and the source tag with + // "Unable to get value of the property 'childNodes': object is null or undefined" + if (!msie || msie > 9) { + they('should NOT require trusted values for $prop src', ['video', 'audio'], + function(tag) { + inject(function($rootScope, $compile, $sce) { + element = $compile('<' + tag + ' src="{{testUrl}}">')($rootScope); + $rootScope.testUrl = 'http://example.com/image.mp4'; + $rootScope.$digest(); + expect(element.attr('src')).toEqual('http://example.com/image.mp4'); + + // But it should accept trusted values anyway. + $rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.mp4'); + $rootScope.$digest(); + expect(element.attr('src')).toEqual('http://example.com/image2.mp4'); + + // and trustedResourceUrls for retrocompatibility + $rootScope.testUrl = $sce.trustAsResourceUrl('http://example.com/image3.mp4'); + $rootScope.$digest(); + expect(element.attr('src')).toEqual('http://example.com/image3.mp4'); + }); + }); + + they('should NOT require trusted values for $prop src', ['source', 'track'], + function(tag) { + inject(function($rootScope, $compile, $sce) { + element = $compile('')($rootScope); + $rootScope.testUrl = 'http://example.com/image.mp4'; + $rootScope.$digest(); + expect(element.find(tag).attr('src')).toEqual('http://example.com/image.mp4'); + + // But it should accept trusted values anyway. + $rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.mp4'); + $rootScope.$digest(); + expect(element.find(tag).attr('src')).toEqual('http://example.com/image2.mp4'); + + // and trustedResourceUrls for retrocompatibility + $rootScope.testUrl = $sce.trustAsResourceUrl('http://example.com/image3.mp4'); + $rootScope.$digest(); + expect(element.find(tag).attr('src')).toEqual('http://example.com/image3.mp4'); + }); + }); + } + }); + + describe('img[src] sanitization', function() { + it('should not sanitize attributes other than src', inject(function($compile, $rootScope) { element = $compile('')($rootScope); $rootScope.testUrl = 'javascript:doEvilStuff()';