fix($compile): reject multi-expression interpolations for src attribute

src/ng/compile.js

@@ -1157,6 +1157,16 @@ function $CompileProvider($provide) {
     }
 
 
+    function isTrustedContext(node, attrNormalizedName) {
+      // maction[xlink:href] can source SVG.  It's not limited to <maction>.
+      if (attrNormalizedName == "xlinkHref" ||
+          (nodeName_(node) != "IMG" && (attrNormalizedName == "src" ||
+                                        attrNormalizedName == "ngSrc"))) {
+        return true;
+      }
+    }
+
+
     function addAttrInterpolateDirective(node, directives, value, name) {
       var interpolateFn = $interpolate(value, true);
 
@@ -1177,7 +1187,7 @@ function $CompileProvider($provide) {
 
           // we need to interpolate again, in case the attribute value has been updated
           // (e.g. by another directive's compile function)
-          interpolateFn = $interpolate(attr[name], true);
+          interpolateFn = $interpolate(attr[name], true, isTrustedContext(node, name));
 
           // if attribute was updated so that there is no interpolation going on we don't want to
           // register any observers

src/ng/interpolate.js

@@ -1,5 +1,7 @@
 'use strict';
 
+var $interpolateMinErr = minErr('$interpolate');
+
 /**
  * @ngdoc object
  * @name ng.$interpolateProvider
@@ -82,14 +84,20 @@ function $InterpolateProvider() {
      * @param {boolean=} mustHaveExpression if set to true then the interpolation string must have
      *    embedded expression in order to return an interpolation function. Strings with no
      *    embedded expression will return null for the interpolation function.
+     * @param {boolean=} isTrustedContext when true, requires that the interpolation string does not
+     *    contain any concatenations - i.e. the interpolation string is a single expression.
+     *    Interpolations for *[src] and *[ng-src] (except IMG, since itwhich sanitizes its value)
+     *    pass true for this parameter.  This helps avoid hunting through the template code to
+     *    figure out of some iframe[src], object[src], etc. was interpolated with a concatenation
+     *    that ended up introducing a XSS.
      * @returns {function(context)} an interpolation function which is used to compute the interpolated
      *    string. The function has these parameters:
      *
      *    * `context`: an object against which any expressions embedded in the strings are evaluated
      *      against.
      *
      */
-    function $interpolate(text, mustHaveExpression) {
+    function $interpolate(text, mustHaveExpression, isTrustedContext) {
       var startIndex,
           endIndex,
           index = 0,
@@ -121,6 +129,18 @@ function $InterpolateProvider() {
         length = 1;
       }
 
+      // Concatenating expressions makes it hard to reason about whether some combination of concatenated
+      // values are unsafe to use and could easily lead to XSS.  By requiring that a single
+      // expression be used for iframe[src], object[src], etc., we ensure that the value that's used
+      // is assigned or constructed by some JS code somewhere that is more testable or make it
+      // obvious that you bound the value to some user controlled value.  This helps reduce the load
+      // when auditing for XSS issues.
+      if (isTrustedContext && parts.length > 1) {
+          throw $interpolateMinErr('noconcat',
+              "Error while interpolating: {0}\nYou may not use multiple expressions when " +
+              "interpolating this expression.", text);
+      }
+
       if (!mustHaveExpression  || hasInterpolation) {
         concat.length = length;
         fn = function(context) {
@@ -139,7 +159,7 @@ function $InterpolateProvider() {
             return concat.join('');
           }
           catch(err) {
-            var newErr = minErr('$interpolate')('interr', "Can't interpolate: {0}\n{1}", text, err.toString());
+            var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text, err.toString());
             $exceptionHandler(newErr);
           }
         };

test/ng/directive/booleanAttrsSpec.js

@@ -89,19 +89,41 @@ describe('boolean attr directives', function() {
 
 describe('ngSrc', function() {
   it('should interpolate the expression and bind to src', inject(function($compile, $rootScope) {
-    var element = $compile('<div ng-src="some/{{id}}"></div>')($rootScope);
+    var element = $compile('<div ng-src="{{id}}"></div>')($rootScope);
 
     $rootScope.$digest();
-    expect(element.attr('src')).toEqual('some/');
+    expect(element.attr('src')).toBeUndefined();
 
     $rootScope.$apply(function() {
       $rootScope.id = 1;
     });
-    expect(element.attr('src')).toEqual('some/1');
+    expect(element.attr('src')).toEqual('1');
 
     dealoc(element);
   }));
 
+  describe('isTrustedContext', function() {
+    it('should NOT interpolate a multi-part expression for non-img src attribute', inject(function($compile, $rootScope) {
+      expect(function() {
+          var element = $compile('<div ng-src="some/{{id}}"></div>')($rootScope);
+          dealoc(element);
+        }).toThrow(
+            "[$interpolate:noconcat] Error while interpolating: some/{{id}}\nYou may not use " +
+            "multiple expressions when interpolating this expression.");
+    }));
+
+    it('should interpolate a multi-part expression for regular attributes', inject(function($compile, $rootScope) {
+      var element = $compile('<div foo="some/{{id}}"></div>')($rootScope);
+      $rootScope.$digest();
+      expect(element.attr('foo')).toBe('some/');
+      $rootScope.$apply(function() {
+        $rootScope.id = 1;
+      });
+      expect(element.attr('foo')).toEqual('some/1');
+    }));
+
+  });
+
   if (msie) {
     it('should update the element property as well as the attribute', inject(
         function($compile, $rootScope) {

test/ng/interpolateSpec.js

@@ -149,6 +149,29 @@ describe('$interpolate', function() {
   });
 
 
+  describe('isTrustedContext', function() {
+    it('should NOT interpolate a multi-part expression when isTrustedContext is true', inject(function($interpolate) {
+      var isTrustedContext = true;
+      expect(function() {
+          $interpolate('constant/{{var}}', true, isTrustedContext);
+        }).toThrow(
+            "[$interpolate:noconcat] Error while interpolating: constant/{{var}}\nYou may not use " +
+            "multiple expressions when interpolating this expression.");
+      expect(function() {
+          $interpolate('{{foo}}{{bar}}', true, isTrustedContext);
+        }).toThrow(
+            "[$interpolate:noconcat] Error while interpolating: {{foo}}{{bar}}\nYou may not use " +
+            "multiple expressions when interpolating this expression.");
+    }));
+
+    it('should interpolate a multi-part expression when isTrustedContext is false', inject(function($interpolate) {
+      expect($interpolate('some/{{id}}')()).toEqual('some/');
+      expect($interpolate('some/{{id}}')({id: 1})).toEqual('some/1');
+      expect($interpolate('{{foo}}{{bar}}')({foo: 1, bar: 2})).toEqual('12');
+    }));
+  });
+
+
   describe('startSymbol', function() {
 
     beforeEach(module(function($interpolateProvider) {
@@ -199,4 +222,5 @@ describe('$interpolate', function() {
       expect($interpolate.endSymbol()).toBe('))');
     }));
   });
+
 });