feat($compile): lower the xlink:href security context for SVG's a and image elements

Previously, `xlink:href` on SVG's `<a>` and `<image>` elements, was
`$sce.RESOURCE_URL`. While this makes sense for other `xlink:href` usecases, it
was an overkill for these elements.
This commit lowers the `xlink:href` security context for these specific
elements, treating it in the same way as `a[href]` or `img[src]` respectively.
The `xlink:href` security context for other elements is not affected.

BREAKING CHANGE:

In the unlikely case that an app relied on RESOURCE_URL whitelisting for the
purpose of binding to the `xlink:href` property of SVG's `<a>` or `<image>`
elements and if the values do not pass the regular URL sanitization, they will
break.

To fix this you need to ensure that the values used for binding to the affected
`xlink:href` contexts are considered safe URLs, e.g. by whitelisting them in
`$compileProvider`'s `aHrefSanitizationWhitelist` (for `<a>` elements) or
`imgSrcSanitizationWhitelist` (for `<image>` elements).

Closes #15736

Author: rjamet

src/ng/compile.js

@@ -1673,9 +1673,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
         nodeName = nodeName_(this.$$element);
 
         if ((nodeName === 'a' && (key === 'href' || key === 'xlinkHref')) ||
-            (nodeName === 'img' && key === 'src')) {
+            (nodeName === 'img' && key === 'src') ||
+            (nodeName === 'image' && key === 'xlinkHref')) {
           // sanitize a[href] and img[src] values
-          this[key] = value = $$sanitizeUri(value, key === 'src');
+          this[key] = value = $$sanitizeUri(value, nodeName === 'img' || nodeName === 'image');
         } else if (nodeName === 'img' && key === 'srcset' && isDefined(value)) {
           // sanitize img[srcset] values
           var result = '';
@@ -3256,8 +3257,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
         if (['img', 'video', 'audio', 'source', 'track'].indexOf(tag) === -1) {
           return $sce.RESOURCE_URL;
         }
-      // maction[xlink:href] can source SVG.  It's not limited to <maction>.
-      } else if (attrNormalizedName === 'xlinkHref' ||
+      } else if (
+          // Some xlink:href are okay, most aren't
+          (attrNormalizedName === 'xlinkHref' && (tag !== 'image' && tag !== 'a')) ||
+          // Formaction
           (tag === 'form' && attrNormalizedName === 'action') ||
           // If relative URLs can go where they are not expected to, then
           // all sorts of trust issues can arise.

test/ng/compileSpec.js

@@ -11122,23 +11122,47 @@ describe('$compile', function() {
     });
 
     it('should use $$sanitizeUri when working with svg and xlink:href', function() {
+      var $$sanitizeUri = jasmine.createSpy('$$sanitizeUri');
+      module(function($provide) {
+        $provide.value('$$sanitizeUri', $$sanitizeUri);
+      });
+      inject(function($compile, $rootScope) {
+        var elementA = $compile('<svg><a xlink:href="{{ testUrl + \'aTag\' }}"></a></svg>')($rootScope);
+        var elementImage = $compile('<svg><image xlink:href="{{ testUrl + \'imageTag\' }}"></image></svg>')($rootScope);
+
+        //both of these fail the RESOURCE_URL test, that shouldn't be run
+        $rootScope.testUrl = 'https://bad.example.org';
+        $$sanitizeUri.and.returnValue('https://clean.example.org');
+
+        $rootScope.$apply();
+        expect(elementA.find('a').attr('xlink:href')).toBe('https://clean.example.org');
+        expect(elementImage.find('image').attr('xlink:href')).toBe('https://clean.example.org');
+        // <a> is navigational, so the second argument should be false to reach the aHref whitelist
+        expect($$sanitizeUri).toHaveBeenCalledWith($rootScope.testUrl + 'aTag' , false);
+        // <image> is media inclusion, it should use the imgSrc whitelist
+        expect($$sanitizeUri).toHaveBeenCalledWith($rootScope.testUrl + 'imageTag', true);
+      });
+    });
+
+    it('should use $$sanitizeUri when working with svg and xlink:href through ng-href', function() {
       var $$sanitizeUri = jasmine.createSpy('$$sanitizeUri');
       module(function($provide) {
         $provide.value('$$sanitizeUri', $$sanitizeUri);
       });
       inject(function($compile, $rootScope) {
         element = $compile('<svg><a xlink:href="" ng-href="{{ testUrl }}"></a></svg>')($rootScope);
-        $rootScope.testUrl = 'evilUrl';
+        //both of these fail the RESOURCE_URL test, that shouldn't be run
+        $rootScope.testUrl = 'https://bad.example.org';
+        $$sanitizeUri.and.returnValue('https://clean.example.org');
 
-        $$sanitizeUri.and.returnValue('someSanitizedUrl');
         $rootScope.$apply();
-        expect(element.find('a').prop('href').baseVal).toBe('someSanitizedUrl');
+        expect(element.find('a').prop('href').baseVal).toBe('https://clean.example.org');
         expect($$sanitizeUri).toHaveBeenCalledWith($rootScope.testUrl, false);
       });
     });
 
 
-    it('should use $$sanitizeUri when working with svg and xlink:href', function() {
+    it('should use $$sanitizeUri when working with svg and xlink:href through ng-href', function() {
       var $$sanitizeUri = jasmine.createSpy('$$sanitizeUri');
       module(function($provide) {
         $provide.value('$$sanitizeUri', $$sanitizeUri);
@@ -11153,6 +11177,17 @@ describe('$compile', function() {
         expect($$sanitizeUri).toHaveBeenCalledWith($rootScope.testUrl, false);
       });
     });
+
+    it('should have a RESOURCE_URL context for xlink:href by default', function() {
+      inject(function($compile, $rootScope) {
+        element = $compile('<svg><whatever xlink:href="{{ testUrl }}"></whatever></svg>')($rootScope);
+        $rootScope.testUrl = 'https://bad.example.org';
+
+        expect(function() {
+          $rootScope.$apply();
+        }).toThrowError(/\$sce:insecurl/);
+      });
+    });
   });
 
   describe('interpolation on HTML DOM event handler attributes onclick, onXYZ, formaction', function() {