fix(icon): static svg urls are trustable (#8484)

angular · May 18, 2016 · 05a3a0f · 05a3a0f
1 parent 4803b49
commit 05a3a0f
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 15 deletions.
diff --git a/src/components/icon/icon.spec.js b/src/components/icon/icon.spec.js
@@ -1,12 +1,11 @@
 describe('MdIcon directive', function() {
-  var el;
-  var $scope;
-  var $compile;
-  var $mdIconProvider;
+  var el, $scope, $compile, $mdIconProvider, $sce;
+  var wasLastSvgSrcTrusted = false;
 
   beforeEach(module('material.components.icon', function(_$mdIconProvider_) {
     $mdIconProvider = _$mdIconProvider_;
   }));
+
   afterEach(function() {
     $mdIconProvider.defaultFontSet('material-icons');
     $mdIconProvider.fontSet('fa', 'fa');
@@ -172,6 +171,13 @@ describe('MdIcon directive', function() {
 
       module(function($provide) {
         var $mdIconMock = function(id) {
+
+          wasLastSvgSrcTrusted = false;
+          if (!angular.isString(id)) {
+            id = $sce.getTrustedUrl(id);
+            wasLastSvgSrcTrusted = true;
+          }
+
           return {
             then: function(fn) {
               switch(id) {
@@ -183,6 +189,8 @@ describe('MdIcon directive', function() {
                   break;
                 case 'cake.svg'         : fn('<svg><g id="cake"></g></svg>');
                   break;
+                case 'galactica.svg'         : fn('<svg><g id="galactica"></g></svg>');
+                  break;
                 case 'image:android'    : fn('');
                   break;
                 default                 :
@@ -229,10 +237,21 @@ describe('MdIcon directive', function() {
 
     describe('using md-svg-src=""', function() {
 
+      beforeEach(inject(function(_$sce_) {
+        $sce = _$sce_;
+      }));
+
+      it('should mark as trusted static URLs', function() {
+        el = make('<md-icon md-svg-src="galactica.svg"></md-icon>');
+        expect(wasLastSvgSrcTrusted).toBe(true);
+        expect(el[0].innerHTML).toContain('galactica')
+      });
+
       it('should update mdSvgSrc when attribute value changes', function() {
         $scope.url = 'android.svg';
         el = make('<md-icon md-svg-src="{{ url }}"></md-icon>');
         expect(el.attr('md-svg-src')).toEqual('android.svg');
+        expect(wasLastSvgSrcTrusted).toBe(false);
         $scope.url = 'cake.svg';
         $scope.$digest();
         expect(el.attr('md-svg-src')).toEqual('cake.svg');

diff --git a/src/components/icon/js/iconDirective.js b/src/components/icon/js/iconDirective.js
@@ -1,6 +1,6 @@
 angular
   .module('material.components.icon')
-  .directive('mdIcon', ['$mdIcon', '$mdTheming', '$mdAria', mdIconDirective]);
+  .directive('mdIcon', ['$mdIcon', '$mdTheming', '$mdAria', '$sce', mdIconDirective]);
 
 /**
  * @ngdoc directive
@@ -171,7 +171,7 @@ angular
  * </hljs>
  *
  */
-function mdIconDirective($mdIcon, $mdTheming, $mdAria ) {
+function mdIconDirective($mdIcon, $mdTheming, $mdAria, $sce) {
 
   return {
     restrict: 'E',
@@ -188,6 +188,10 @@ function mdIconDirective($mdIcon, $mdTheming, $mdAria ) {
 
     prepareForFontIcon();
 
+    // Keep track of the content of the svg src so we can compare against it later to see if the
+    // attribute is static (and thus safe).
+    var originalSvgSrc = element[0].getAttribute(attr.$attr.mdSvgSrc);
+
     // If using a font-icon, then the textual name of the icon itself
     // provides the aria-label.
 
@@ -213,6 +217,12 @@ function mdIconDirective($mdIcon, $mdTheming, $mdAria ) {
       // Use either pre-configured SVG or URL source, respectively.
       attr.$observe(attrName, function(attrVal) {
 
+        // If using svg-src and the value is static (i.e., is exactly equal to the compile-time
+        // `md-svg-src` value), then it is implicitly trusted.
+        if (!isInlineSvg(attrVal) && attrVal === originalSvgSrc) {
+          attrVal = $sce.trustAsUrl(attrVal);
+        }
+
         element.empty();
         if (attrVal) {
           $mdIcon(attrVal)
@@ -245,4 +255,14 @@ function mdIconDirective($mdIcon, $mdTheming, $mdAria ) {
       }
     }
   }
+
+  /**
+   * Gets whether the given svg src is an inline ("data:" style) SVG.
+   * @param {string} svgSrc The svg src.
+   * @returns {boolean} Whether the src is an inline SVG.
+   */
+  function isInlineSvg(svgSrc) {
+    var dataUrlRegex = /^data:image\/svg\+xml[\s*;\w\-\=]*?(base64)?,(.*)$/i;
+    return dataUrlRegex.test(svgSrc);
+  }
 }
diff --git a/src/components/icon/js/iconService.js b/src/components/icon/js/iconService.js
@@ -358,9 +358,9 @@ MdIconProvider.prototype = {
 
   },
 
-  $get: ['$templateRequest', '$q', '$log', '$templateCache', '$mdUtil', function($templateRequest, $q, $log, $templateCache, $mdUtil) {
+  $get: ['$templateRequest', '$q', '$log', '$templateCache', '$mdUtil', '$sce', function($templateRequest, $q, $log, $templateCache, $mdUtil, $sce) {
     this.preloadIcons($templateCache);
-    return MdIconService(config, $templateRequest, $q, $log, $mdUtil);
+    return MdIconService(config, $templateRequest, $q, $log, $mdUtil, $sce);
   }]
 };
 
@@ -415,7 +415,7 @@ function ConfigurationItem(url, viewBoxSize) {
  */
 
 /* @ngInject */
-function MdIconService(config, $templateRequest, $q, $log, $mdUtil) {
+function MdIconService(config, $templateRequest, $q, $log, $mdUtil, $sce) {
   var iconCache = {};
   var urlRegex = /[-\w@:%\+.~#?&//=]{2,}\.[a-z]{2,4}\b(\/[-\w@:%\+.~#?&//=]*)?/i;
   var dataUrlRegex = /^data:image\/svg\+xml[\s*;\w\-\=]*?(base64)?,(.*)$/i;
@@ -432,12 +432,28 @@ function MdIconService(config, $templateRequest, $q, $log, $mdUtil) {
   function getIcon(id) {
     id = id || '';
 
+    // If the "id" provided is not a string, the only other valid value is a $sce trust wrapper
+    // over a URL string. If the value is not trusted, this will intentionally throw an error
+    // because the user is attempted to use an unsafe URL, potentially opening themselves up
+    // to an XSS attack.
+    if (!angular.isString(id)) {
+      id = $sce.getTrustedUrl(id);
+    }
+
     // If already loaded and cached, use a clone of the cached icon.
     // Otherwise either load by URL, or lookup in the registry and then load by URL, and cache.
 
-    if (iconCache[id]) return $q.when(transformClone(iconCache[id]));
-    if (urlRegex.test(id) || dataUrlRegex.test(id)) return loadByURL(id).then(cacheIcon(id));
-    if (id.indexOf(':') == -1) id = '$default:' + id;
+    if (iconCache[id]) {
+      return $q.when(transformClone(iconCache[id]));
+    }
+
+    if (urlRegex.test(id) || dataUrlRegex.test(id)) {
+      return loadByURL(id).then(cacheIcon(id));
+    }
+
+    if (id.indexOf(':') == -1) {
+      id = '$default:' + id;
+    }
 
     var load = config[id] ? loadByID : loadFromIconSet;
     return load(id)
@@ -540,9 +556,7 @@ function MdIconService(config, $templateRequest, $q, $log, $mdUtil) {
     /* Load the icon by URL using HTTP. */
     function loadByHttpUrl(url) {
       return $q(function(resolve, reject) {
-        /**
-         * Catch HTTP or generic errors not related to incorrect icon IDs.
-         */
+        // Catch HTTP or generic errors not related to incorrect icon IDs.
         var announceAndReject = function(err) {
             var msg = angular.isString(err) ? err : (err.message || err.data || err.statusText);
             $log.warn(msg);