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
rjamet wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b43909
feat($compile): Lower the security context of SVG's a and image xlink…
rjamet Feb 21, 2017
0a22243
fix($compile): image xlink:href should be sanitized as images, not as…
rjamet Mar 3, 2017
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
Prev Previous commit
fix($compile): image xlink:href should be sanitized as images, not as… 
… links.
  • Loading branch information
@rjamet
rjamet committed Mar 3, 2017
commit 0a22243d3d1a8c192359a1e09a6a2660f75089e6
3 changes: 2 additions & 1 deletion src/ng/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1674,7 +1674,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
(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');
Copy link
Member

@gkalpak gkalpak Mar 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: There is no need to wrap this line (as it does not exceed 100 chars).

} else if (nodeName === 'img' && key === 'srcset' && isDefined(value)) {
// sanitize img[srcset] values
var result = '';
Expand Down
12 changes: 8 additions & 4 deletions test/ng/compileSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11127,15 +11127,20 @@ describe('$compile', function() {
$provide.value('$$sanitizeUri', $$sanitizeUri);
});
inject(function($compile, $rootScope) {
element = $compile('<svg><a xlink:href="{{ testUrl }}"></a></svg>')($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(element.find('a').attr('xlink:href')).toBe('https://clean.example.org');
expect($$sanitizeUri).toHaveBeenCalledWith($rootScope.testUrl, false);
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);
});
});

Expand Down Expand Up @@ -11173,7 +11178,6 @@ describe('$compile', function() {
});
});


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);
Expand Down