feat($sce): handle URLs through the $sce service.

This is a large patch to handle URLs with the $sce service, similarly to HTML context.
However, to keep rough compatibility with existing apps, we need to allow URL-context
concatenation, since previously $sce contexts prevented sanitization. There's now a
set of special contexts (defined in $interpolate) that allow concatenation, in a roughly
intuitive way: trusted types alone are trusted, and any concatenation of values results
in a non-trusted type that will be handled by getTrusted once the concatenation is done.
Thus, "{{safeType}}" will not be sanitized, "{{ 'javascript:foo' }}" will be, and
"javascript:{{safeType}}" will also be.

There's backwards incompatible changes in there, mostly the fusion of the two URL
context sanitization whitelists (the separation is nice, but not important for
security).

Author: rjamet

src/ng/compile.js

@@ -1119,14 +1119,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
 
   /**
    * @ngdoc method
-   * @name $compileProvider#aHrefSanitizationWhitelist
+   * @name $compileProvider#uriSanitizationWhitelist
    * @kind function
    *
    * @description
    * Retrieves or overrides the default regular expression that is used for whitelisting of safe
-   * urls during a[href] sanitization.
+   * urls during URL-context sanitization.
    *
-   * The sanitization is a security measure aimed at preventing XSS attacks via html links.
+   * The sanitization is a security measure aimed at preventing XSS attacks.
    *
    * Any url about to be assigned to a[href] via data-binding is first normalized and turned into
    * an absolute url. Afterwards, the url is matched against the `aHrefSanitizationWhitelist`
@@ -1137,45 +1137,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
    * @returns {RegExp|ng.$compileProvider} Current RegExp if called without value or self for
    *    chaining otherwise.
    */
-  this.aHrefSanitizationWhitelist = function(regexp) {
+  this.uriSanitizationWhitelist = function(regexp) {
     if (isDefined(regexp)) {
-      $$sanitizeUriProvider.aHrefSanitizationWhitelist(regexp);
+      $$sanitizeUriProvider.uriSanitizationWhitelist(regexp);
       return this;
     } else {
-      return $$sanitizeUriProvider.aHrefSanitizationWhitelist();
+      return $$sanitizeUriProvider.uriSanitizationWhitelist();
     }
   };
 
 
-  /**
-   * @ngdoc method
-   * @name $compileProvider#imgSrcSanitizationWhitelist
-   * @kind function
-   *
-   * @description
-   * Retrieves or overrides the default regular expression that is used for whitelisting of safe
-   * urls during img[src] sanitization.
-   *
-   * The sanitization is a security measure aimed at prevent XSS attacks via html links.
-   *
-   * Any url about to be assigned to img[src] via data-binding is first normalized and turned into
-   * an absolute url. Afterwards, the url is matched against the `imgSrcSanitizationWhitelist`
-   * regular expression. If a match is found, the original url is written into the dom. Otherwise,
-   * the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM.
-   *
-   * @param {RegExp=} regexp New regexp to whitelist urls with.
-   * @returns {RegExp|ng.$compileProvider} Current RegExp if called without value or self for
-   *    chaining otherwise.
-   */
-  this.imgSrcSanitizationWhitelist = function(regexp) {
-    if (isDefined(regexp)) {
-      $$sanitizeUriProvider.imgSrcSanitizationWhitelist(regexp);
-      return this;
-    } else {
-      return $$sanitizeUriProvider.imgSrcSanitizationWhitelist();
-    }
-  };
-
   /**
    * @ngdoc method
    * @name  $compileProvider#debugInfoEnabled
@@ -1209,9 +1180,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
 
   this.$get = [
             '$injector', '$interpolate', '$exceptionHandler', '$templateRequest', '$parse',
-            '$controller', '$rootScope', '$sce', '$animate', '$$sanitizeUri',
+            '$controller', '$rootScope', '$sce', '$animate',
     function($injector,   $interpolate,   $exceptionHandler,   $templateRequest,   $parse,
-             $controller,   $rootScope,   $sce,   $animate,   $$sanitizeUri) {
+             $controller,   $rootScope,   $sce,   $animate) {
 
     var SIMPLE_ATTR_NAME = /^\w/;
     var specialAttrHolder = document.createElement('div');
@@ -1350,11 +1321,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
 
         nodeName = nodeName_(this.$$element);
 
-        if ((nodeName === 'a' && (key === 'href' || key === 'xlinkHref')) ||
-            (nodeName === 'img' && key === 'src')) {
-          // sanitize a[href] and img[src] values
-          this[key] = value = $$sanitizeUri(value, key === 'src');
-        } else if (nodeName === 'img' && key === 'srcset') {
+        // img[srcset] is a bit too weird of a beast to handle through $sce.
+        // Instead, for now at least, sanitize each of the URIs individually.
+        // That works even dynamically, but it's not bypassable through the $sce.
+        // Instead, if you want several unsafe URLs as-is, you should probably
+        // use trustAsHtml on the whole tag.
+        if (nodeName === 'img' && key === 'srcset') {
+
           // sanitize img[srcset] values
           var result = "";
 
@@ -1372,16 +1345,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           for (var i = 0; i < nbrUrisWith2parts; i++) {
             var innerIdx = i * 2;
             // sanitize the uri
-            result += $$sanitizeUri(trim(rawUris[innerIdx]), true);
+            result += $sce.getTrustedUrl(trim(rawUris[innerIdx]));
             // add the descriptor
-            result += (" " + trim(rawUris[innerIdx + 1]));
+            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);
+          result += $sce.getTrustedUrl(trim(lastTuple[0]));
 
           // and add the last descriptor if any
           if (lastTuple.length === 2) {
@@ -2815,6 +2788,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           (tag != "img" && (attrNormalizedName == "src" ||
                             attrNormalizedName == "ngSrc"))) {
         return $sce.RESOURCE_URL;
+      } else if (attrNormalizedName == "src" ||
+          attrNormalizedName == "href" ||
+          attrNormalizedName == "ngHref" ||
+          attrNormalizedName == "ngSrc") {
+        // All that is not a RESOURCE_URL but still a URL. This might be overkill for some
+        // attributes, but the sanitization is permissive, so it should not be troublesome.
+        return $sce.URL;
       }
     }
 

src/ng/interpolate.js

@@ -4,8 +4,8 @@ var $interpolateMinErr = angular.$interpolateMinErr = minErr('$interpolate');
 $interpolateMinErr.throwNoconcat = function(text) {
   throw $interpolateMinErr('noconcat',
       "Error while interpolating: {0}\nStrict Contextual Escaping disallows " +
-      "interpolations that concatenate multiple expressions when a trusted value is " +
-      "required.  See http://docs.angularjs.org/api/ng.$sce", text);
+      "interpolations that concatenate multiple expressions in some secure contexts. " +
+      "See http://docs.angularjs.org/api/ng.$sce", text);
 };
 
 $interpolateMinErr.interr = function(text, err) {
@@ -137,6 +137,10 @@ function $InterpolateProvider() {
       }, listener, objectEquality);
     }
 
+    function isConcatenationAllowed(context) {
+        return context == $sce.URL;
+    }
+
     /**
      * @ngdoc service
      * @name $interpolate
@@ -254,7 +258,9 @@ function $InterpolateProvider() {
           textLength = text.length,
           exp,
           concat = [],
-          expressionPositions = [];
+          expressionPositions = [],
+          singleExpression = false,
+          contextAllowsConcatenation = isConcatenationAllowed(trustedContext);
 
       while (index < textLength) {
         if (((startIndex = text.indexOf(startSymbol, index)) != -1) &&
@@ -267,7 +273,7 @@ function $InterpolateProvider() {
           parseFns.push($parse(exp, parseStringifyInterceptor));
           index = endIndex + endSymbolLength;
           expressionPositions.push(concat.length);
-          concat.push('');
+          concat.push(''); // Placeholder that will get replaced with the evaluated expression.
         } else {
           // we did not find an interpolation, so we have to add the remainder to the separators array
           if (index !== textLength) {
@@ -277,27 +283,54 @@ function $InterpolateProvider() {
         }
       }
 
+      if (concat.length == 1 && expressionPositions.length == 1) {
+        singleExpression = true;
+      }
+
       // 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 (trustedContext && concat.length > 1) {
-          $interpolateMinErr.throwNoconcat(text);
-      }
+      // single expression be used for some $sce-managed secure contexts (RESOURCE_URLs mostly),
+      // 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.
+      // Note that URL-context is dispensed of this, since its getTrusted method can sanitize.
+      // In that context, .getTrusted will be called on either the single expression or on the
+      // overall concatenated string (losing trusted types used in the mix, by design). Both these
+      // methods will sanitize plain strings. Also, HTML could be included, but since it's only used
+      // in srcdoc attributes, this would not be very useful.
 
       if (!mustHaveExpression || expressions.length) {
         var compute = function(values) {
           for (var i = 0, ii = expressions.length; i < ii; i++) {
             if (allOrNothing && isUndefined(values[i])) return;
             concat[expressionPositions[i]] = values[i];
           }
-          return concat.join('');
+
+          if (contextAllowsConcatenation) {
+            if (singleExpression) {
+              // The raw value was left as-is by parseStringifyInterceptor
+              return $sce.getTrusted(trustedContext, concat[0]);
+            } else {
+              return $sce.getTrusted(trustedContext, concat.join(''));
+            }
+          } else if (trustedContext) {
+            if (concat.length > 1) {
+              // there's at least two parts, so expr + string or exp + exp, and this context
+              // doesn't allow that.
+              $interpolateMinErr.throwNoconcat(text);
+            } else {
+              return concat.join('');
+            }
+          } else { // In an unprivileged context, just concatenate and return.
+            return concat.join('');
+          }
         };
 
         var getValue = function(value) {
-          return trustedContext ?
+          // In concatenable contexts, getTrusted comes at the end, to avoid sanitizing individual
+          // parts of a full URL. We don't care about losing the trustedness here, that's handled in
+          // parseStringifyInterceptor below.
+          return (trustedContext && !contextAllowsConcatenation) ?
             $sce.getTrusted(trustedContext, value) :
             $sce.valueOf(value);
         };
@@ -314,7 +347,7 @@ function $InterpolateProvider() {
 
               return compute(values);
             } catch (err) {
-              $exceptionHandler($interpolateMinErr.interr(text, err));
+              $exceptionHandler(err);
             }
 
           }, {
@@ -336,8 +369,13 @@ function $InterpolateProvider() {
 
       function parseStringifyInterceptor(value) {
         try {
-          value = getValue(value);
-          return allOrNothing && !isDefined(value) ? value : stringify(value);
+          if (contextAllowsConcatenation && singleExpression) {
+            // No stringification in this case, to keep the trusted value until unwrapping.
+            return value;
+          } else {
+            value = getValue(value);
+            return allOrNothing && !isDefined(value) ? value : stringify(value);
+          }
         } catch (err) {
           $exceptionHandler($interpolateMinErr.interr(text, err));
         }

src/ng/sanitizeUri.js

@@ -2,67 +2,41 @@
 
 /**
  * @description
- * Private service to sanitize uris for links and images. Used by $compile and $sanitize.
+ * Private service to sanitize uris for $sce.URL context. Used by $compile, $sce and $sanitize.
  */
 function $$SanitizeUriProvider() {
-  var aHrefSanitizationWhitelist = /^\s*(https?|ftp|mailto|tel|file):/,
-    imgSrcSanitizationWhitelist = /^\s*((https?|ftp|file|blob):|data:image\/)/;
-
-  /**
-   * @description
-   * Retrieves or overrides the default regular expression that is used for whitelisting of safe
-   * urls during a[href] sanitization.
-   *
-   * The sanitization is a security measure aimed at prevent XSS attacks via html links.
-   *
-   * Any url about to be assigned to a[href] via data-binding is first normalized and turned into
-   * an absolute url. Afterwards, the url is matched against the `aHrefSanitizationWhitelist`
-   * regular expression. If a match is found, the original url is written into the dom. Otherwise,
-   * the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM.
-   *
-   * @param {RegExp=} regexp New regexp to whitelist urls with.
-   * @returns {RegExp|ng.$compileProvider} Current RegExp if called without value or self for
-   *    chaining otherwise.
-   */
-  this.aHrefSanitizationWhitelist = function(regexp) {
-    if (isDefined(regexp)) {
-      aHrefSanitizationWhitelist = regexp;
-      return this;
-    }
-    return aHrefSanitizationWhitelist;
-  };
+  var uriSanitizationWhitelist = /^\s*((https?|ftp|file|blob|tel|mailto):|data:image\/)/i;
 
 
   /**
    * @description
    * Retrieves or overrides the default regular expression that is used for whitelisting of safe
-   * urls during img[src] sanitization.
+   * urls.
    *
-   * The sanitization is a security measure aimed at prevent XSS attacks via html links.
+   * The sanitization is a security measure aimed at prevent XSS attacks.
    *
-   * Any url about to be assigned to img[src] via data-binding is first normalized and turned into
-   * an absolute url. Afterwards, the url is matched against the `imgSrcSanitizationWhitelist`
+   * Any url about to be assigned to URL context via data-binding is first normalized and turned into
+   * an absolute url. Afterwards, the url is matched against the `urlSanitizationWhitelist`
    * regular expression. If a match is found, the original url is written into the dom. Otherwise,
-   * the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM.
+   * the absolute url is prefixed with `'unsafe:'` string and only then is it written into the DOM,
+   * making it inactive.
    *
    * @param {RegExp=} regexp New regexp to whitelist urls with.
    * @returns {RegExp|ng.$compileProvider} Current RegExp if called without value or self for
    *    chaining otherwise.
    */
-  this.imgSrcSanitizationWhitelist = function(regexp) {
+  this.uriSanitizationWhitelist = function(regexp) {
     if (isDefined(regexp)) {
-      imgSrcSanitizationWhitelist = regexp;
+      uriSanitizationWhitelist = regexp;
       return this;
     }
-    return imgSrcSanitizationWhitelist;
+    return uriSanitizationWhitelist;
   };
 
   this.$get = function() {
-    return function sanitizeUri(uri, isImage) {
-      var regex = isImage ? imgSrcSanitizationWhitelist : aHrefSanitizationWhitelist;
-      var normalizedVal;
-      normalizedVal = urlResolve(uri).href;
-      if (normalizedVal !== '' && !normalizedVal.match(regex)) {
+    return function sanitizeUri(uri) {
+      var normalizedVal = urlResolve(uri).href;
+      if (normalizedVal !== '' && !normalizedVal.match(uriSanitizationWhitelist)) {
         return 'unsafe:' + normalizedVal;
       }
       return uri;