diff --git a/Gruntfile.js b/Gruntfile.js index de0d6c72c8cd..264fe874b776 100644 --- 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 index 694bde47c0a9..dc98bcc18648 100755 --- 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 index 25e21b2c5b4e..7745cce935d9 100755 --- 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 index fc54adcf0bc2..9e642ac2d65f 100644 --- 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 index ab3d2208f680..9a5a5af60906 100644 --- 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() { * *
  *     
- *     
+ *
*
* - * 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): * *
- *   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 (`
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() {
- -
Error: {{myCtrl.errorMsg}}
-
-
- {{userComment.name}}: - +

+ User comments
+ 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. +
+
+ {{userComment.name}}: + +
+
-
- // 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( + 'Hover over this text.'); }); [ { "name": "Alice", - "htmlComment": "Is anyone reading this?" + "htmlComment": "Is anyone reading this?" }, { "name": "Bob", "htmlComment": "Yes! Am I the only other one?" @@ -521,14 +511,15 @@ function $SceDelegateProvider() { - describe('SCE doc demo', function() { - it('should bind trusted values', function() { - element('#fetchBtn').click(); - expect(element('.htmlComment').html()).toBe('Is anyone 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('Is anyone reading this?'); + }); + it('should NOT sanitize explicitly trusted values', function() { + expect(element('#explicitlyTrustedHtml').html()).toBe( + 'Hover over this text.'); + }); }); diff --git a/src/ngSanitize/directive/ngBindHtml.js b/src/ngSanitize/directive/ngBindHtml.js deleted file mode 100644 index 150e6bdc1e21..000000000000 --- a/src/ngSanitize/directive/ngBindHtml.js +++ /dev/null @@ -1,25 +0,0 @@ -'use strict'; - - -/** - * @ngdoc directive - * @name ngSanitize.directive:ngBindHtml - * - * @description - * Creates a binding that will sanitize the result of evaluating the `expression` with the - * {@link ngSanitize.$sanitize $sanitize} service and innerHTML the result into the current element. - * - * See {@link ngSanitize.$sanitize $sanitize} docs for examples. - * - * @element ANY - * @param {expression} ngBindHtml {@link guide/expression Expression} to evaluate. - */ -angular.module('ngSanitize').directive('ngBindHtml', ['$sanitize', function($sanitize) { - return function(scope, element, attr) { - element.addClass('ng-binding').data('$binding', attr.ngBindHtml); - scope.$watch(attr.ngBindHtml, function ngBindHtmlWatchAction(value) { - value = $sanitize(value); - element.html(value || ''); - }); - }; -}]); diff --git a/src/ngSanitize/sanitize.js b/src/ngSanitize/sanitize.js index 110b3a64f682..70932899c9a8 100644 --- a/src/ngSanitize/sanitize.js +++ b/src/ngSanitize/sanitize.js @@ -68,8 +68,7 @@ var ngSanitizeMinErr = angular.$$minErr('ngSanitize'); '

an html\n' + 'click here\n' + 'snippet

'; - // 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: - + + - - - - + + + + + - - + + + + + + + + + - - - - -
FilterDirectiveHow Source Rendered
html filter -
<div ng-bind-html="snippet">
</div>
-
-
-
ng-bind-htmlAutomatically uses $sanitize
<div ng-bind-html="snippet">
</div>
no filter
ng-bind-htmlBypass $sanitize by explicitly trusting the dangerous value
<div ng-bind-html="deliberatelyTrustDangerousSnippet()">
</div>
ng-bindAutomatically escapes
<div ng-bind="snippet">
</div>
unsafe html filter
<div ng-bind-html-unsafe="getSceSnippet()">
</div>
- 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('

an html\nclick here\nsnippet

'); }); + it('should inline raw snippet if bound to a trusted value', function() { + expect(using('#bind-html-with-trust').element("div").html()). + toBe("

an html\n" + + "click here\n" + + "snippet

"); + }); + it('should escape snippet without any filter', function() { - expect(using('#escaped-html').element('div').html()). + expect(using('#bind-default').element('div').html()). toBe("<p style=\"color:blue\">an html\n" + "<em onmouseover=\"this.textContent='PWN3D!'\">click here</em>\n" + "snippet</p>"); }); - it('should inline raw snippet if filtered as unsafe', function() { - expect(using('#html-unsafe-filter').element("div").html()). - toBe("

an html\n" + - "click here\n" + - "snippet

"); - }); - - it('should update', function($sce) { - input('snippet').enter('new text'); - expect(using('#html-filter').binding('snippet')).toBe('new text'); - expect(using('#escaped-html').element('div').html()).toBe("new <b>text</b>"); - expect(using('#html-unsafe-filter').element('div').html()).toBe('new text'); + it('should update', function() { + input('snippet').enter('new text'); + expect(using('#bind-html-with-sanitize').element('div').html()).toBe('new text'); + expect(using('#bind-html-with-trust').element('div').html()).toBe('new text'); + expect(using('#bind-default').element('div').html()).toBe("new <b onclick=\"alert(1)\">text</b>"); });
diff --git a/test/ng/directive/ngBindSpec.js b/test/ng/directive/ngBindSpec.js index 1d8f8ef4d906..be68464ffc1a 100644 --- a/test/ng/directive/ngBindSpec.js +++ b/test/ng/directive/ngBindSpec.js @@ -67,19 +67,14 @@ describe('ngBind*', function() { }); - describe('ngBindHtmlUnsafe', function() { - - function configureSce(enabled) { - module(function($provide, $sceProvider) { - $sceProvider.enabled(enabled); - }); - }; - + describe('ngBindHtml', function() { describe('SCE disabled', function() { - beforeEach(function() {configureSce(false)}); + beforeEach(function() { + module(function($sceProvider) { $sceProvider.enabled(false); }); + }); - it('should set unsafe html', inject(function($rootScope, $compile) { - element = $compile('
')($rootScope); + it('should set html', inject(function($rootScope, $compile) { + element = $compile('
')($rootScope); $rootScope.html = '
hello
'; $rootScope.$digest(); expect(angular.lowercase(element.html())).toEqual('
hello
'); @@ -88,27 +83,35 @@ describe('ngBind*', function() { describe('SCE enabled', function() { - beforeEach(function() {configureSce(true)}); - - it('should NOT set unsafe html for untrusted values', inject(function($rootScope, $compile) { - element = $compile('
')($rootScope); + it('should NOT set html for untrusted values', inject(function($rootScope, $compile) { + element = $compile('
')($rootScope); $rootScope.html = '
hello
'; expect($rootScope.$digest).toThrow(); })); - it('should NOT set unsafe html for wrongly typed values', inject(function($rootScope, $compile, $sce) { - element = $compile('
')($rootScope); + it('should NOT set html for wrongly typed values', inject(function($rootScope, $compile, $sce) { + element = $compile('
')($rootScope); $rootScope.html = $sce.trustAsCss('
hello
'); expect($rootScope.$digest).toThrow(); })); - it('should set unsafe html for trusted values', inject(function($rootScope, $compile, $sce) { - element = $compile('
')($rootScope); + it('should set html for trusted values', inject(function($rootScope, $compile, $sce) { + element = $compile('
')($rootScope); $rootScope.html = $sce.trustAsHtml('
hello
'); $rootScope.$digest(); expect(angular.lowercase(element.html())).toEqual('
hello
'); })); + describe('when $sanitize is available', function() { + beforeEach(function() { module('ngSanitize'); }); + + it('should sanitize untrusted html', inject(function($rootScope, $compile) { + element = $compile('
')($rootScope); + $rootScope.html = '
hello
'; + $rootScope.$digest(); + expect(angular.lowercase(element.html())).toEqual('
hello
'); + })); + }); }); }); diff --git a/test/ng/sceSpecs.js b/test/ng/sceSpecs.js index 16525b8d02f0..6157fc17f70d 100644 --- a/test/ng/sceSpecs.js +++ b/test/ng/sceSpecs.js @@ -341,7 +341,22 @@ describe('SCE', function() { expect(function() { $sce.getTrustedResourceUrl('open_redirect'); }).toThrow( '[$sce:isecrurl] Blocked loading resource from url not allowed by $sceDelegate policy. URL: open_redirect'); })); + }); + + describe('sanitizing html', function() { + describe('when $sanitize is NOT available', function() { + it('should throw an exception for getTrusted(string) values', inject(function($sce) { + expect(function() { $sce.getTrustedHtml(''); }).toThrow( + '[$sce:unsafe] Attempting to use an unsafe value in a safe context.'); + })); + }); + describe('when $sanitize is available', function() { + beforeEach(function() { module('ngSanitize'); }); + it('should sanitize html using $sanitize', inject(function($sce) { + expect($sce.getTrustedHtml('abc')).toBe('abc'); + })); + }); }); });