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

feat($compile): Lower the security context of SVG's a and image xlink:href #15736

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1671,9 +1671,11 @@ 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 = '';
Expand Down Expand Up @@ -3254,8 +3256,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.
Expand Down
43 changes: 39 additions & 4 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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() {
Expand Down