Skip to content

Commit ab80cd9

Browse files
ltrillaudjeffbcross
ltrillaud
authored and
jeffbcross
committed
fix(compile): sanitize srcset attribute
Applies similar sanitization as is applie to img[src] to img[srcset], while adapting to the different semantics and syntax of srcset.
1 parent 8199f4d commit ab80cd9

File tree

4 files changed

+135
-8
lines changed

4 files changed

+135
-8
lines changed

‎src/ng/compile.js‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -882,10 +882,44 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
882882

883883
nodeName = nodeName_(this.$$element);
884884

885-
// sanitize a[href] and img[src] values
886885
if ((nodeName === 'a' && key === 'href') ||
887886
(nodeName === 'img' && key === 'src')) {
887+
// sanitize a[href] and img[src] values
888888
this[key] = value = $$sanitizeUri(value, key === 'src');
889+
} else if (nodeName === 'img' && key === 'srcset') {
890+
// sanitize img[srcset] values
891+
var result = "";
892+
893+
// first check if there are spaces because it's not the same pattern
894+
var trimmedSrcset = trim(value);
895+
// ( 999x ,| 999w ,| ,|, )
896+
var srcPattern = /(\s+\d+x\s*,|\s+\d+w\s*,|\s+,|,\s+)/;
897+
var pattern = /\s/.test(trimmedSrcset) ? srcPattern : /(,)/;
898+
899+
// split srcset into tuple of uri and descriptor except for the last item
900+
var rawUris = trimmedSrcset.split(pattern);
901+
902+
// for each tuples
903+
var nbrUrisWith2parts = Math.floor(rawUris.length / 2);
904+
for (var i=0; i<nbrUrisWith2parts; i++) {
905+
var innerIdx = i*2;
906+
// sanitize the uri
907+
result += $$sanitizeUri(trim( rawUris[innerIdx]), true);
908+
// add the descriptor
909+
result += ( " " + trim(rawUris[innerIdx+1]));
910+
}
911+
912+
// split the last item into uri and descriptor
913+
var lastTuple = trim(rawUris[i*2]).split(/\s/);
914+
915+
// sanitize the last uri
916+
result += $$sanitizeUri(trim(lastTuple[0]), true);
917+
918+
// and add the last descriptor if any
919+
if( lastTuple.length === 2) {
920+
result += (" " + trim(lastTuple[1]));
921+
}
922+
this[key] = value = result;
889923
}
890924

891925
if (writeAttr !== false) {

‎test/ng/compileSpec.js‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5870,6 +5870,74 @@ describe('$compile', function() {
58705870
});
58715871
});
58725872

5873+
describe('img[srcset] sanitization', function() {
5874+
5875+
it('should NOT require trusted values for img srcset', inject(function($rootScope, $compile, $sce) {
5876+
element = $compile('<img srcset="{{testUrl}}"></img>')($rootScope);
5877+
$rootScope.testUrl = 'http://example.com/image.png';
5878+
$rootScope.$digest();
5879+
expect(element.attr('srcset')).toEqual('http://example.com/image.png');
5880+
// But it should accept trusted values anyway.
5881+
$rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.png');
5882+
$rootScope.$digest();
5883+
expect(element.attr('srcset')).toEqual('http://example.com/image2.png');
5884+
}));
5885+
5886+
it('should use $$sanitizeUri', function() {
5887+
var $$sanitizeUri = jasmine.createSpy('$$sanitizeUri');
5888+
module(function($provide) {
5889+
$provide.value('$$sanitizeUri', $$sanitizeUri);
5890+
});
5891+
inject(function($compile, $rootScope) {
5892+
element = $compile('<img srcset="{{testUrl}}"></img>')($rootScope);
5893+
$rootScope.testUrl = "someUrl";
5894+
5895+
$$sanitizeUri.andReturn('someSanitizedUrl');
5896+
$rootScope.$apply();
5897+
expect(element.attr('srcset')).toBe('someSanitizedUrl');
5898+
expect($$sanitizeUri).toHaveBeenCalledWith($rootScope.testUrl, true);
5899+
});
5900+
});
5901+
5902+
it('should sanitize all uris in srcset', inject(function($rootScope, $compile) {
5903+
/*jshint scripturl:true*/
5904+
element = $compile('<img srcset="{{testUrl}}"></img>')($rootScope);
5905+
var testSet = {
5906+
'http://example.com/image.png':'http://example.com/image.png',
5907+
' http://example.com/image.png':'http://example.com/image.png',
5908+
'http://example.com/image.png ':'http://example.com/image.png',
5909+
'http://example.com/image.png 128w':'http://example.com/image.png 128w',
5910+
'http://example.com/image.png 2x':'http://example.com/image.png 2x',
5911+
'http://example.com/image.png 1.5x':'http://example.com/image.png 1.5x',
5912+
'http://example.com/image1.png 1x,http://example.com/image2.png 2x':'http://example.com/image1.png 1x,http://example.com/image2.png 2x',
5913+
'http://example.com/image1.png 1x ,http://example.com/image2.png 2x':'http://example.com/image1.png 1x ,http://example.com/image2.png 2x',
5914+
'http://example.com/image1.png 1x, http://example.com/image2.png 2x':'http://example.com/image1.png 1x,http://example.com/image2.png 2x',
5915+
'http://example.com/image1.png 1x , http://example.com/image2.png 2x':'http://example.com/image1.png 1x ,http://example.com/image2.png 2x',
5916+
'http://example.com/image1.png 48w,http://example.com/image2.png 64w':'http://example.com/image1.png 48w,http://example.com/image2.png 64w',
5917+
//Test regex to make sure doesn't mistake parts of url for width descriptors
5918+
'http://example.com/image1.png?w=48w,http://example.com/image2.png 64w':'http://example.com/image1.png?w=48w,http://example.com/image2.png 64w',
5919+
'http://example.com/image1.png 1x,http://example.com/image2.png 64w':'http://example.com/image1.png 1x,http://example.com/image2.png 64w',
5920+
'http://example.com/image1.png,http://example.com/image2.png':'http://example.com/image1.png ,http://example.com/image2.png',
5921+
'http://example.com/image1.png ,http://example.com/image2.png':'http://example.com/image1.png ,http://example.com/image2.png',
5922+
'http://example.com/image1.png, http://example.com/image2.png':'http://example.com/image1.png ,http://example.com/image2.png',
5923+
'http://example.com/image1.png , http://example.com/image2.png':'http://example.com/image1.png ,http://example.com/image2.png',
5924+
'http://example.com/image1.png 1x, http://example.com/image2.png 2x, http://example.com/image3.png 3x':
5925+
'http://example.com/image1.png 1x,http://example.com/image2.png 2x,http://example.com/image3.png 3x',
5926+
'javascript:doEvilStuff() 2x': 'unsafe:javascript:doEvilStuff() 2x',
5927+
'http://example.com/image1.png 1x,javascript:doEvilStuff() 2x':'http://example.com/image1.png 1x,unsafe:javascript:doEvilStuff() 2x',
5928+
'http://example.com/image1.jpg?x=a,b 1x,http://example.com/ima,ge2.jpg 2x':'http://example.com/image1.jpg?x=a,b 1x,http://example.com/ima,ge2.jpg 2x',
5929+
//Test regex to make sure doesn't mistake parts of url for pixel density descriptors
5930+
'http://example.com/image1.jpg?x=a2x,b 1x,http://example.com/ima,ge2.jpg 2x':'http://example.com/image1.jpg?x=a2x,b 1x,http://example.com/ima,ge2.jpg 2x'
5931+
};
5932+
5933+
forEach( testSet, function( ref, url) {
5934+
$rootScope.testUrl = url;
5935+
$rootScope.$digest();
5936+
expect(element.attr('srcset')).toEqual(ref);
5937+
});
5938+
5939+
}));
5940+
});
58735941

58745942
describe('a[href] sanitization', function() {
58755943

‎test/ng/directive/ngSrcSpec.js‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,22 @@ describe('ngSrc', function() {
88
dealoc(element);
99
});
1010

11-
it('should not result empty string in img src', inject(function($rootScope, $compile) {
12-
$rootScope.image = {};
13-
element = $compile('<img ng-src="{{image.url}}">')($rootScope);
14-
$rootScope.$digest();
15-
expect(element.attr('src')).not.toBe('');
16-
expect(element.attr('src')).toBe(undefined);
17-
}));
11+
describe('img[ng-src]', function() {
12+
it('should not result empty string in img src', inject(function($rootScope, $compile) {
13+
$rootScope.image = {};
14+
element = $compile('<img ng-src="{{image.url}}">')($rootScope);
15+
$rootScope.$digest();
16+
expect(element.attr('src')).not.toBe('');
17+
expect(element.attr('src')).toBe(undefined);
18+
}));
19+
20+
it('should sanitize url', inject(function($rootScope, $compile) {
21+
$rootScope.imageUrl = 'javascript:alert(1);';
22+
element = $compile('<img ng-src="{{imageUrl}}">')($rootScope);
23+
$rootScope.$digest();
24+
expect(element.attr('src')).toBe('unsafe:javascript:alert(1);');
25+
}));
26+
});
1827

1928
describe('iframe[ng-src]', function() {
2029
it('should pass through src attributes for the same domain', inject(function($compile, $rootScope) {

‎test/ng/directive/ngSrcsetSpec.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/*jshint scripturl:true*/
12
'use strict';
23

34
describe('ngSrcset', function() {
@@ -13,4 +14,19 @@ describe('ngSrcset', function() {
1314
$rootScope.$digest();
1415
expect(element.attr('srcset')).toBeUndefined();
1516
}));
17+
18+
it('should sanitize good urls', inject(function($rootScope, $compile) {
19+
$rootScope.imageUrl = 'http://example.com/image1.png 1x, http://example.com/image2.png 2x';
20+
element = $compile('<img ng-srcset="{{imageUrl}}">')($rootScope);
21+
$rootScope.$digest();
22+
expect(element.attr('srcset')).toBe('http://example.com/image1.png 1x,http://example.com/image2.png 2x');
23+
}));
24+
25+
it('should sanitize evil url', inject(function($rootScope, $compile) {
26+
$rootScope.imageUrl = 'http://example.com/image1.png 1x, javascript:doEvilStuff() 2x';
27+
element = $compile('<img ng-srcset="{{imageUrl}}">')($rootScope);
28+
$rootScope.$digest();
29+
expect(element.attr('srcset')).toBe('http://example.com/image1.png 1x,unsafe:javascript:doEvilStuff() 2x');
30+
}));
1631
});
32+

0 commit comments

Comments
 (0)