feat(ngBindHtml, sce): combine ng-bind-html and ng-bind-html-unsafe

Changes: - remove ng-bind-html-unsafe - ng-bind-html is now in core - ng-bind-html is secure - supports SCE - so you can bind to an arbitrary trusted string - automatic sanitization if $sanitize is available BREAKING CHANGE: ng-html-bind-unsafe has been removed and replaced by ng-html-bind (which has been removed from ngSanitize.) ng-bind-html provides ng-html-bind-unsafe like behavior (innerHTML's the result without sanitization) when bound to the result of $sce.trustAsHtml(string). When bound to a plain string, the string is sanitized via $sanitize before being innerHTML'd. If $sanitize isn't available, it's logs an exception.
angular · Jul 25, 2013 · dae6947 · dae6947
1 parent bea9422
commit dae6947
Show file tree

Hide file tree

Showing 9 changed files with 134 additions and 149 deletions.
diff --git a/Gruntfile.js b/Gruntfile.js
@@ -153,7 +153,6 @@ module.exports = function(grunt) {
         dest: 'build/angular-sanitize.js',
         src: util.wrap([
           'src/ngSanitize/sanitize.js',
-          'src/ngSanitize/directive/ngBindHtml.js',
           'src/ngSanitize/filter/linky.js'
         ], 'module')
       },

diff --git a/angularFiles.js b/angularFiles.js
@@ -73,7 +73,6 @@ angularFiles = {
     'src/ngRoute/routeParams.js',
     'src/ngRoute/directive/ngView.js',
     'src/ngSanitize/sanitize.js',
-    'src/ngSanitize/directive/ngBindHtml.js',
     'src/ngSanitize/filter/linky.js',
     'src/ngMock/angular-mocks.js',
     'src/ngMobile/mobile.js',

diff --git a/src/AngularPublic.js b/src/AngularPublic.js
@@ -72,7 +72,7 @@ function publishExternalAPI(angular){
             style: styleDirective,
             option: optionDirective,
             ngBind: ngBindDirective,
-            ngBindHtmlUnsafe: ngBindHtmlUnsafeDirective,
+            ngBindHtml: ngBindHtmlDirective,
             ngBindTemplate: ngBindTemplateDirective,
             ngClass: ngClassDirective,
             ngClassEven: ngClassEvenDirective,

diff --git a/src/ng/directive/ngBind.js b/src/ng/directive/ngBind.js
@@ -116,23 +116,27 @@ var ngBindTemplateDirective = ['$interpolate', function($interpolate) {
 
 /**
  * @ngdoc directive
- * @name ng.directive:ngBindHtmlUnsafe
+ * @name ng.directive:ngBindHtml
  *
  * @description
  * Creates a binding that will innerHTML the result of evaluating the `expression` into the current
- * element. *The innerHTML-ed content will not be sanitized!* You should use this directive only if
- * {@link ngSanitize.directive:ngBindHtml ngBindHtml} directive is too
- * restrictive and when you absolutely trust the source of the content you are binding to.
+ * element in a secure way.  By default, the innerHTML-ed content will be sanitized using the {@link
+ * ngSanitize.$sanitize $sanitize} service.  To utilize this functionality, ensure that `$sanitize`
+ * is available, for example, by including {@link ngSanitize} in your module's dependencies (not in
+ * core Angular.)  You may also bypass sanitization for values you know are safe. To do so, bind to
+ * an explicitly trusted value via {@link ng.$sce#trustAsHtml $sce.trustAsHtml}.  See the example
+ * under {@link ng.$sce#Example Strict Contextual Escaping (SCE)}.
  *
- * See {@link ngSanitize.$sanitize $sanitize} docs for examples.
+ * Note: If a `$sanitize` service is unavailable and the bound value isn't explicitly trusted, you
+ * will have an exception (instead of an exploit.)
  *
  * @element ANY
- * @param {expression} ngBindHtmlUnsafe {@link guide/expression Expression} to evaluate.
+ * @param {expression} ngBindHtml {@link guide/expression Expression} to evaluate.
  */
-var ngBindHtmlUnsafeDirective = ['$sce', function($sce) {
+var ngBindHtmlDirective = ['$sce', function($sce) {
   return function(scope, element, attr) {
-    element.addClass('ng-binding').data('$binding', attr.ngBindHtmlUnsafe);
-    scope.$watch($sce.parseAsHtml(attr.ngBindHtmlUnsafe), function ngBindHtmlUnsafeWatchAction(value) {
+    element.addClass('ng-binding').data('$binding', attr.ngBindHtml);
+    scope.$watch($sce.parseAsHtml(attr.ngBindHtml), function ngBindHtmlWatchAction(value) {
       element.html(value || '');
     });
   };

diff --git a/src/ng/sce.js b/src/ng/sce.js
@@ -137,8 +137,17 @@ function $SceDelegateProvider() {
             (documentProtocol === "http:" && resourceProtocol === "https:"));
   }
 
-  this.$get = ['$log', '$document', '$$urlUtils', function(
-                $log,   $document,   $$urlUtils) {
+  this.$get = ['$log', '$document', '$injector', '$$urlUtils', function(
+                $log,   $document,   $injector,   $$urlUtils) {
+
+    var htmlSanitizer = function htmlSanitizer(html) {
+      throw $sceMinErr('unsafe', 'Attempting to use an unsafe value in a safe context.');
+    };
+
+    if ($injector.has('$sanitize')) {
+      htmlSanitizer = $injector.get('$sanitize');
+    }
+
 
     function matchUrl(matcher, parsedUrl) {
       if (matcher === 'self') {
@@ -285,6 +294,9 @@ function $SceDelegateProvider() {
       if (constructor && maybeTrusted instanceof constructor) {
         return maybeTrusted.$$unwrapTrustedValue();
       }
+      // If we get here, then we may only take one of two actions.
+      // 1. sanitize the value for the requested type, or
+      // 2. throw an exception.
       if (type === SCE_CONTEXTS.RESOURCE_URL) {
         if (isResourceUrlAllowedByPolicy(maybeTrusted)) {
           return maybeTrusted;
@@ -293,6 +305,8 @@ function $SceDelegateProvider() {
               'Blocked loading resource from url not allowed by $sceDelegate policy.  URL: {0}', maybeTrusted.toString());
           return;
         }
+      } else if (type === SCE_CONTEXTS.HTML) {
+        return htmlSanitizer(maybeTrusted);
       }
       throw $sceMinErr('unsafe', 'Attempting to use an unsafe value in a safe context.');
     }
@@ -329,8 +343,8 @@ function $SceDelegateProvider() {
  *
  * Strict Contextual Escaping (SCE) is a mode in which AngularJS requires bindings in certain
  * contexts to result in a value that is marked as safe to use for that context One example of such
- * a context is binding arbitrary html controlled by the user via `ng-bind-html-unsafe`.  We refer
- * to these contexts as privileged or SCE contexts.
+ * a context is binding arbitrary html controlled by the user via `ng-bind-html`.  We refer to these
+ * contexts as privileged or SCE contexts.
  *
  * As of version 1.2, Angular ships with SCE enabled by default.
  *
@@ -347,10 +361,10 @@ function $SceDelegateProvider() {
  *
  * <pre class="prettyprint">
  *     <input ng-model="userHtml">
- *     <div ng-bind-html-unsafe="{{userHtml}}">
+ *     <div ng-bind-html="{{userHtml}}">
  * </pre>
  *
- * Notice that `ng-bind-html-unsafe` is bound to `{{userHtml}}` controlled by the user.  With SCE
+ * Notice that `ng-bind-html` is bound to `{{userHtml}}` controlled by the user.  With SCE
  * disabled, this application allows the user to render arbitrary HTML into the DIV.
  * In a more realistic example, one may be rendering user comments, blog articles, etc. via
  * bindings.  (HTML is just one example of a context where rendering user controlled input creates
@@ -384,14 +398,14 @@ function $SceDelegateProvider() {
  * ng.$sce#parse $sce.parseAs} rather than `$parse` to watch attribute bindings, which performs the
  * {@link ng.$sce#getTrusted $sce.getTrusted} behind the scenes on non-constant literals.
  *
- * As an example, {@link ng.directive:ngBindHtmlUnsafe ngBindHtmlUnsafe} uses {@link
+ * As an example, {@link ng.directive:ngBindHtml ngBindHtml} uses {@link
  * ng.$sce#parseHtml $sce.parseAsHtml(binding expression)}.  Here's the actual code (slightly
  * simplified):
  *
  * <pre class="prettyprint">
- *   var ngBindHtmlUnsafeDirective = ['$sce', function($sce) {
+ *   var ngBindHtmlDirective = ['$sce', function($sce) {
  *     return function(scope, element, attr) {
- *       scope.$watch($sce.parseAsHtml(attr.ngBindHtmlUnsafe), function(value) {
+ *       scope.$watch($sce.parseAsHtml(attr.ngBindHtml), function(value) {
  *         element.html(value || '');
  *       });
  *     };
@@ -444,7 +458,7 @@ function $SceDelegateProvider() {
  *
  * | Context             | Notes          |
  * |=====================|================|
- * | `$sce.HTML`         | For HTML that's safe to source into the application.  The {@link ng.directive:ngBindHtmlUnsafe ngBindHtmlUnsafe} directive uses this context for bindings. |
+ * | `$sce.HTML`         | For HTML that's safe to source into the application.  The {@link ng.directive:ngBindHtml ngBindHtml} directive uses this context for bindings. |
  * | `$sce.CSS`          | For CSS that's safe to source into the application.  Currently unused.  Feel free to use it in your own directives. |
  * | `$sce.URL`          | For URLs that are safe to follow as links.  Currently unused (`<a href=` and `<img src=` sanitize their urls and don't consititute an SCE context. |
  * | `$sce.RESOURCE_URL` | For URLs that are not only safe to follow as links, but whose contens are also safe to include in your application.  Examples include `ng-include`, `src` / `ngSrc` bindings for tags other than `IMG` (e.g. `IFRAME`, `OBJECT`, etc.)  <br><br>Note that `$sce.RESOURCE_URL` makes a stronger statement about the URL than `$sce.URL` does and therefore contexts requiring values trusted for `$sce.RESOURCE_URL` can be used anywhere that values trusted for `$sce.URL` are required. |
@@ -458,61 +472,37 @@ function $SceDelegateProvider() {
  <example module="mySceApp">
   <file name="index.html">
     <div ng-controller="myAppController as myCtrl">
-      <button ng-click="myCtrl.fetchUserComments()" id="fetchBtn">Fetch Comments</button>
-      <div ng-show="myCtrl.errorMsg">Error: {{myCtrl.errorMsg}}</div>
-      <div ng-repeat="userComment in myCtrl.userComments">
-        <hr>
-        <b>{{userComment.name}}</b>:
-        <span ng-bind-html-unsafe="userComment.htmlComment" class="htmlComment"></span>
+      <i ng-bind-html="myCtrl.explicitlyTrustedHtml" id="explicitlyTrustedHtml"></i><br><br>
+      <b>User comments</b><br>
+      By default, HTML that isn't explicitly trusted (e.g. Alice's comment) is sanitized when $sanitize is available.  If $sanitize isn't available, this results in an error instead of an exploit.
+      <div class="well">
+        <div ng-repeat="userComment in myCtrl.userComments">
+          <b>{{userComment.name}}</b>:
+          <span ng-bind-html="userComment.htmlComment" class="htmlComment"></span>
+          <br>
+        </div>
       </div>
-      <div ng-bind-html-unsafe="myCtrl.someHtml" id="someHtml"></div>
     </div>
   </file>
 
   <file name="script.js">
-    // These types of functions would be in the data access layer of your application code.
-    function fetchUserCommentsFromServer($http, $q, $templateCache, $sce) {
-      var deferred = $q.defer();
-      $http({method: "GET", url: "test_data.json", cache: $templateCache}).
-        success(function(userComments, status) {
-          // The comments coming from the server have been sanitized by the server and can be
-          // trusted.
-          angular.forEach(userComments, function(userComment) {
-            userComment.htmlComment = $sce.trustAsHtml(userComment.htmlComment);
-          });
-          deferred.resolve(userComments);
-        }).
-        error(function (data, status) {
-          deferred.reject("HTTP status code " + status + ": " + data);
-        });
-      return deferred.promise;
-    };
-
-    var mySceApp = angular.module('mySceApp', []);
+    var mySceApp = angular.module('mySceApp', ['ngSanitize']);
 
-    mySceApp.controller("myAppController", function myAppController($injector) {
+    mySceApp.controller("myAppController", function myAppController($http, $templateCache, $sce) {
       var self = this;
-
-      self.someHtml = "This might have been any binding including an input element " +
-                      "controlled by the user.";
-
-      self.fetchUserComments = function() {
-        $injector.invoke(fetchUserCommentsFromServer).then(
-            function onSuccess(userComments) {
-              self.errorMsg = null;
-              self.userComments = userComments;
-            },
-            function onFailure(errorMsg) {
-              self.errorMsg = errorMsg;
-            });
-      }
+      $http.get("test_data.json", {cache: $templateCache}).success(function(userComments) {
+        self.userComments = userComments;
+      });
+      self.explicitlyTrustedHtml = $sce.trustAsHtml(
+          '<span onmouseover="this.textContent=&quot;Explicitly trusted HTML bypasses ' +
+          'sanitization.&quot;">Hover over this text.</span>');
     });
   </file>
 
   <file name="test_data.json">
     [
       { "name": "Alice",
-        "htmlComment": "Is <i>anyone</i> reading this?"
+        "htmlComment": "<span onmouseover='this.textContent=\"PWN3D!\"'>Is <i>anyone</i> reading this?</span>"
       },
       { "name": "Bob",
         "htmlComment": "<i>Yes!</i>  Am I the only other one?"
@@ -521,14 +511,15 @@ function $SceDelegateProvider() {
   </file>
 
   <file name="scenario.js">
-     describe('SCE doc demo', function() {
-       it('should bind trusted values', function() {
-         element('#fetchBtn').click();
-         expect(element('.htmlComment').html()).toBe('Is <i>anyone</i> reading this?');
-       });
-       it('should NOT bind arbitrary values', function() {
-         expect(element('#someHtml').html()).toBe('');
-       });
+    describe('SCE doc demo', function() {
+      it('should sanitize untrusted values', function() {
+        expect(element('.htmlComment').html()).toBe('<span>Is <i>anyone</i> reading this?</span>');
+      });
+      it('should NOT sanitize explicitly trusted values', function() {
+        expect(element('#explicitlyTrustedHtml').html()).toBe(
+            '<span onmouseover="this.textContent=&quot;Explicitly trusted HTML bypasses ' +
+            'sanitization.&quot;">Hover over this text.</span>');
+      });
     });
   </file>
  </example>

diff --git a/src/ngSanitize/directive/ngBindHtml.js b/src/ngSanitize/directive/ngBindHtml.js
diff --git a/src/ngSanitize/sanitize.js b/src/ngSanitize/sanitize.js
@@ -68,8 +68,7 @@ var ngSanitizeMinErr = angular.$$minErr('ngSanitize');
              '<p style="color:blue">an html\n' +
              '<em onmouseover="this.textContent=\'PWN3D!\'">click here</em>\n' +
              'snippet</p>';
-           // ng-bind-html-unsafe requires a $sce trusted value of type $sce.HTML.
-           $scope.getSceSnippet = function() {
+           $scope.deliberatelyTrustDangerousSnippet = function() {
              return $sce.trustAsHtml($scope.snippet);
            };
          }
@@ -78,57 +77,57 @@ var ngSanitizeMinErr = angular.$$minErr('ngSanitize');
           Snippet: <textarea ng-model="snippet" cols="60" rows="3"></textarea>
            <table>
              <tr>
-               <td>Filter</td>
+               <td>Directive</td>
+               <td>How</td>
                <td>Source</td>
                <td>Rendered</td>
              </tr>
-             <tr id="html-filter">
-               <td>html filter</td>
-               <td>
-                 <pre>&lt;div ng-bind-html="snippet"&gt;<br/>&lt;/div&gt;</pre>
-               </td>
-               <td>
-                 <div ng-bind-html="snippet"></div>
-               </td>
+             <tr id="bind-html-with-sanitize">
+               <td>ng-bind-html</td>
+               <td>Automatically uses $sanitize</td>
+               <td><pre>&lt;div ng-bind-html="snippet"&gt;<br/>&lt;/div&gt;</pre></td>
+               <td><div ng-bind-html="snippet"></div></td>
              </tr>
-             <tr id="escaped-html">
-               <td>no filter</td>
+             <tr id="bind-html-with-trust">
+               <td>ng-bind-html</td>
+               <td>Bypass $sanitize by explicitly trusting the dangerous value</td>
+               <td><pre>&lt;div ng-bind-html="deliberatelyTrustDangerousSnippet()"&gt;<br/>&lt;/div&gt;</pre></td>
+               <td><div ng-bind-html="deliberatelyTrustDangerousSnippet()"></div></td>
+             </tr>
+             <tr id="bind-default">
+               <td>ng-bind</td>
+               <td>Automatically escapes</td>
                <td><pre>&lt;div ng-bind="snippet"&gt;<br/>&lt;/div&gt;</pre></td>
                <td><div ng-bind="snippet"></div></td>
              </tr>
-             <tr id="html-unsafe-filter">
-               <td>unsafe html filter</td>
-               <td><pre>&lt;div ng-bind-html-unsafe="getSceSnippet()"&gt;<br/>&lt;/div&gt;</pre></td>
-               <td><div ng-bind-html-unsafe="getSceSnippet()"></div></td>
-             </tr>
            </table>
          </div>
      </doc:source>
      <doc:scenario>
-       it('should sanitize the html snippet ', function() {
-         expect(using('#html-filter').element('div').html()).
+       it('should sanitize the html snippet by default', function() {
+         expect(using('#bind-html-with-sanitize').element('div').html()).
            toBe('<p>an html\n<em>click here</em>\nsnippet</p>');
        });
 
+       it('should inline raw snippet if bound to a trusted value', function() {
+         expect(using('#bind-html-with-trust').element("div").html()).
+           toBe("<p style=\"color:blue\">an html\n" +
+                "<em onmouseover=\"this.textContent='PWN3D!'\">click here</em>\n" +
+                "snippet</p>");
+       });
+
        it('should escape snippet without any filter', function() {
-         expect(using('#escaped-html').element('div').html()).
+         expect(using('#bind-default').element('div').html()).
            toBe("&lt;p style=\"color:blue\"&gt;an html\n" +
                 "&lt;em onmouseover=\"this.textContent='PWN3D!'\"&gt;click here&lt;/em&gt;\n" +
                 "snippet&lt;/p&gt;");
        });
 
-       it('should inline raw snippet if filtered as unsafe', function() {
-         expect(using('#html-unsafe-filter').element("div").html()).
-           toBe("<p style=\"color:blue\">an html\n" +
-                "<em onmouseover=\"this.textContent='PWN3D!'\">click here</em>\n" +
-                "snippet</p>");
-       });
-
-       it('should update', function($sce) {
-         input('snippet').enter('new <b>text</b>');
-         expect(using('#html-filter').binding('snippet')).toBe('new <b>text</b>');
-         expect(using('#escaped-html').element('div').html()).toBe("new &lt;b&gt;text&lt;/b&gt;");
-         expect(using('#html-unsafe-filter').element('div').html()).toBe('new <b>text</b>');
+       it('should update', function() {
+         input('snippet').enter('new <b onclick="alert(1)">text</b>');
+         expect(using('#bind-html-with-sanitize').element('div').html()).toBe('new <b>text</b>');
+         expect(using('#bind-html-with-trust').element('div').html()).toBe('new <b onclick="alert(1)">text</b>');
+         expect(using('#bind-default').element('div').html()).toBe("new &lt;b onclick=\"alert(1)\"&gt;text&lt;/b&gt;");
        });
      </doc:scenario>
    </doc:example>