From ab80cd90661396dbb1c94c5f4dd2d11ee8f6b6af Mon Sep 17 00:00:00 2001
From: ltrillaud <ltrillaud@jouve.fr>
Date: Mon, 8 Sep 2014 11:03:28 +0200
Subject: [PATCH] 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.
---
 src/ng/compile.js                 | 36 +++++++++++++++-
 test/ng/compileSpec.js            | 68 +++++++++++++++++++++++++++++++
 test/ng/directive/ngSrcSpec.js    | 23 +++++++----
 test/ng/directive/ngSrcsetSpec.js | 16 ++++++++
 4 files changed, 135 insertions(+), 8 deletions(-)

diff --git a/src/ng/compile.js b/src/ng/compile.js
index 95d07518011b..fd3c228e3de7 100644
--- a/src/ng/compile.js
+++ b/src/ng/compile.js
@@ -882,10 +882,44 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
 
         nodeName = nodeName_(this.$$element);
 
-        // sanitize a[href] and img[src] values
         if ((nodeName === 'a' && key === 'href') ||
             (nodeName === 'img' && key === 'src')) {
+          // sanitize a[href] and img[src] values
           this[key] = value = $$sanitizeUri(value, key === 'src');
+        } else if (nodeName === 'img' && key === 'srcset') {
+          // sanitize img[srcset] values
+          var result = "";
+
+          // first check if there are spaces because it's not the same pattern
+          var trimmedSrcset = trim(value);
+          //                (   999x   ,|   999w   ,|   ,|,   )
+          var srcPattern = /(\s+\d+x\s*,|\s+\d+w\s*,|\s+,|,\s+)/;
+          var pattern = /\s/.test(trimmedSrcset) ? srcPattern : /(,)/;
+
+          // split srcset into tuple of uri and descriptor except for the last item
+          var rawUris = trimmedSrcset.split(pattern);
+
+          // for each tuples
+          var nbrUrisWith2parts = Math.floor(rawUris.length / 2);
+          for (var i=0; i<nbrUrisWith2parts; i++) {
+            var innerIdx = i*2;
+            // sanitize the uri
+            result += $$sanitizeUri(trim( rawUris[innerIdx]), true);
+            // add the descriptor
+            result += ( " " + trim(rawUris[innerIdx+1]));
+          }
+
+          // split the last item into uri and descriptor
+          var lastTuple = trim(rawUris[i*2]).split(/\s/);
+
+          // sanitize the last uri
+          result += $$sanitizeUri(trim(lastTuple[0]), true);
+
+          // and add the last descriptor if any
+          if( lastTuple.length === 2) {
+            result += (" " + trim(lastTuple[1]));
+          }
+          this[key] = value = result;
         }
 
         if (writeAttr !== false) {
diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
index 8514e6470a07..816aa4a7b503 100755
--- a/test/ng/compileSpec.js
+++ b/test/ng/compileSpec.js
@@ -5870,6 +5870,74 @@ describe('$compile', function() {
     });
   });
 
+  describe('img[srcset] sanitization', function() {
+
+    it('should NOT require trusted values for img srcset', inject(function($rootScope, $compile, $sce) {
+      element = $compile('<img srcset="{{testUrl}}"></img>')($rootScope);
+      $rootScope.testUrl = 'http://example.com/image.png';
+      $rootScope.$digest();
+      expect(element.attr('srcset')).toEqual('http://example.com/image.png');
+      // But it should accept trusted values anyway.
+      $rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.png');
+      $rootScope.$digest();
+      expect(element.attr('srcset')).toEqual('http://example.com/image2.png');
+    }));
+
+    it('should use $$sanitizeUri', function() {
+      var $$sanitizeUri = jasmine.createSpy('$$sanitizeUri');
+      module(function($provide) {
+        $provide.value('$$sanitizeUri', $$sanitizeUri);
+      });
+      inject(function($compile, $rootScope) {
+        element = $compile('<img srcset="{{testUrl}}"></img>')($rootScope);
+        $rootScope.testUrl = "someUrl";
+
+        $$sanitizeUri.andReturn('someSanitizedUrl');
+        $rootScope.$apply();
+        expect(element.attr('srcset')).toBe('someSanitizedUrl');
+        expect($$sanitizeUri).toHaveBeenCalledWith($rootScope.testUrl, true);
+      });
+    });
+
+    it('should sanitize all uris in srcset', inject(function($rootScope, $compile) {
+      /*jshint scripturl:true*/
+      element = $compile('<img srcset="{{testUrl}}"></img>')($rootScope);
+      var testSet = {
+        'http://example.com/image.png':'http://example.com/image.png',
+        ' http://example.com/image.png':'http://example.com/image.png',
+        'http://example.com/image.png ':'http://example.com/image.png',
+        'http://example.com/image.png 128w':'http://example.com/image.png 128w',
+        'http://example.com/image.png 2x':'http://example.com/image.png 2x',
+        'http://example.com/image.png 1.5x':'http://example.com/image.png 1.5x',
+        'http://example.com/image1.png 1x,http://example.com/image2.png 2x':'http://example.com/image1.png 1x,http://example.com/image2.png 2x',
+        'http://example.com/image1.png 1x ,http://example.com/image2.png 2x':'http://example.com/image1.png 1x ,http://example.com/image2.png 2x',
+        'http://example.com/image1.png 1x, http://example.com/image2.png 2x':'http://example.com/image1.png 1x,http://example.com/image2.png 2x',
+        'http://example.com/image1.png 1x , http://example.com/image2.png 2x':'http://example.com/image1.png 1x ,http://example.com/image2.png 2x',
+        'http://example.com/image1.png 48w,http://example.com/image2.png 64w':'http://example.com/image1.png 48w,http://example.com/image2.png 64w',
+        //Test regex to make sure doesn't mistake parts of url for width descriptors
+        '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',
+        'http://example.com/image1.png 1x,http://example.com/image2.png 64w':'http://example.com/image1.png 1x,http://example.com/image2.png 64w',
+        'http://example.com/image1.png,http://example.com/image2.png':'http://example.com/image1.png ,http://example.com/image2.png',
+        'http://example.com/image1.png ,http://example.com/image2.png':'http://example.com/image1.png ,http://example.com/image2.png',
+        'http://example.com/image1.png, http://example.com/image2.png':'http://example.com/image1.png ,http://example.com/image2.png',
+        'http://example.com/image1.png , http://example.com/image2.png':'http://example.com/image1.png ,http://example.com/image2.png',
+        'http://example.com/image1.png 1x, http://example.com/image2.png 2x, http://example.com/image3.png 3x':
+          'http://example.com/image1.png 1x,http://example.com/image2.png 2x,http://example.com/image3.png 3x',
+        'javascript:doEvilStuff() 2x': 'unsafe:javascript:doEvilStuff() 2x',
+        'http://example.com/image1.png 1x,javascript:doEvilStuff() 2x':'http://example.com/image1.png 1x,unsafe:javascript:doEvilStuff() 2x',
+        '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',
+        //Test regex to make sure doesn't mistake parts of url for pixel density descriptors
+        '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'
+      };
+
+      forEach( testSet, function( ref, url) {
+        $rootScope.testUrl = url;
+        $rootScope.$digest();
+        expect(element.attr('srcset')).toEqual(ref);
+      });
+
+    }));
+  });
 
   describe('a[href] sanitization', function() {
 
diff --git a/test/ng/directive/ngSrcSpec.js b/test/ng/directive/ngSrcSpec.js
index 8f21d6e941b3..4b8b14bd46f7 100644
--- a/test/ng/directive/ngSrcSpec.js
+++ b/test/ng/directive/ngSrcSpec.js
@@ -8,13 +8,22 @@ describe('ngSrc', function() {
     dealoc(element);
   });
 
-  it('should not result empty string in img src', inject(function($rootScope, $compile) {
-    $rootScope.image = {};
-    element = $compile('<img ng-src="{{image.url}}">')($rootScope);
-    $rootScope.$digest();
-    expect(element.attr('src')).not.toBe('');
-    expect(element.attr('src')).toBe(undefined);
-  }));
+  describe('img[ng-src]', function() {
+    it('should not result empty string in img src', inject(function($rootScope, $compile) {
+      $rootScope.image = {};
+      element = $compile('<img ng-src="{{image.url}}">')($rootScope);
+      $rootScope.$digest();
+      expect(element.attr('src')).not.toBe('');
+      expect(element.attr('src')).toBe(undefined);
+    }));
+
+    it('should sanitize url', inject(function($rootScope, $compile) {
+      $rootScope.imageUrl = 'javascript:alert(1);';
+      element = $compile('<img ng-src="{{imageUrl}}">')($rootScope);
+      $rootScope.$digest();
+      expect(element.attr('src')).toBe('unsafe:javascript:alert(1);');
+    }));
+  });
 
   describe('iframe[ng-src]', function() {
     it('should pass through src attributes for the same domain', inject(function($compile, $rootScope) {
diff --git a/test/ng/directive/ngSrcsetSpec.js b/test/ng/directive/ngSrcsetSpec.js
index a95a108b9ba6..8d14ca5f1b79 100644
--- a/test/ng/directive/ngSrcsetSpec.js
+++ b/test/ng/directive/ngSrcsetSpec.js
@@ -1,3 +1,4 @@
+/*jshint scripturl:true*/
 'use strict';
 
 describe('ngSrcset', function() {
@@ -13,4 +14,19 @@ describe('ngSrcset', function() {
     $rootScope.$digest();
     expect(element.attr('srcset')).toBeUndefined();
   }));
+
+  it('should sanitize good urls', inject(function($rootScope, $compile) {
+    $rootScope.imageUrl = 'http://example.com/image1.png 1x, http://example.com/image2.png 2x';
+    element = $compile('<img ng-srcset="{{imageUrl}}">')($rootScope);
+    $rootScope.$digest();
+    expect(element.attr('srcset')).toBe('http://example.com/image1.png 1x,http://example.com/image2.png 2x');
+  }));
+
+  it('should sanitize evil url', inject(function($rootScope, $compile) {
+    $rootScope.imageUrl = 'http://example.com/image1.png 1x, javascript:doEvilStuff() 2x';
+    element = $compile('<img ng-srcset="{{imageUrl}}">')($rootScope);
+    $rootScope.$digest();
+    expect(element.attr('srcset')).toBe('http://example.com/image1.png 1x,unsafe:javascript:doEvilStuff() 2x');
+  }));
 });
+