From 181fc567d873df065f1e84af7225deb70a8d2eb9 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Fri, 7 Aug 2015 12:39:03 -0700 Subject: [PATCH] feat($sanitize): make svg support an opt-in Closes #12524 BREAKING CHANGE: The svg support in is now an opt-in option Applications that depend on this option can use to turn the option back on, but while doing so, please read the warning provided in the documentation for information on preventing click-hijacking attacks when this option is turned on. --- src/ngSanitize/sanitize.js | 71 ++++++++++++++++++++++++--- test/ngSanitize/filter/linkySpec.js | 6 ++- test/ngSanitize/sanitizeSpec.js | 76 +++++++++++++++++++---------- 3 files changed, 118 insertions(+), 35 deletions(-) diff --git a/src/ngSanitize/sanitize.js b/src/ngSanitize/sanitize.js index 70797a1c9267..8cf7b9e2b474 100644 --- a/src/ngSanitize/sanitize.js +++ b/src/ngSanitize/sanitize.js @@ -34,13 +34,17 @@ var $sanitizeMinErr = angular.$$minErr('$sanitize'); * @kind function * * @description + * Sanitizes an html string by stripping all potentially dangerous tokens. + * * The input is sanitized by parsing the HTML into tokens. All safe tokens (from a whitelist) are * then serialized back to properly escaped html string. This means that no unsafe input can make - * it into the returned string, however, since our parser is more strict than a typical browser - * parser, it's possible that some obscure input, which would be recognized as valid HTML by a - * browser, won't make it through the sanitizer. The input may also contain SVG markup. - * The whitelist is configured using the functions `aHrefSanitizationWhitelist` and - * `imgSrcSanitizationWhitelist` of {@link ng.$compileProvider `$compileProvider`}. + * it into the returned string. + * + * The whitelist for URL sanitization of attribute values is configured using the functions + * `aHrefSanitizationWhitelist` and `imgSrcSanitizationWhitelist` of {@link ng.$compileProvider + * `$compileProvider`}. + * + * The input may also contain SVG markup if this is enabled via {@link $sanitizeProvider}. * * @param {string} html HTML input. * @returns {string} Sanitized HTML. @@ -126,8 +130,22 @@ var $sanitizeMinErr = angular.$$minErr('$sanitize'); */ + + +/** + * @ngdoc provider + * @name $sanitizeProvider + * + * @description + * Creates and configures {@link $sanitize} instance. + */ function $SanitizeProvider() { + var svgEnabled = false; + this.$get = ['$$sanitizeUri', function($$sanitizeUri) { + if (svgEnabled) { + angular.extend(validElements, svgElements); + } return function(html) { var buf = []; htmlParser(html, htmlSanitizeWriter(buf, function(uri, isImage) { @@ -136,6 +154,46 @@ function $SanitizeProvider() { return buf.join(''); }; }]; + + + /** + * @ngdoc method + * @name $sanitizeProvider#enableSvg + * @kind function + * + * @description + * Enables a subset of svg to be supported by the sanitizer. + * + *
+ *

By enabling this setting without taking other precautions, you might expose your + * application to click-hijacking attacks. In these attacks, sanitized svg elements could be positioned + * outside of the containing element and be rendered over other elements on the page (e.g. a login + * link). Such behavior can then result in phishing incidents.

+ * + *

To protect against these, explicitly setup `overflow: hidden` css rule for all potential svg + * tags within the sanitized content:

+ * + *
+ * + * 

+   *   .rootOfTheIncludedContent svg {
+   *     overflow: hidden !important;
+   *   }
+   *
+ *
+ * + * @param {boolean=} regexp New regexp to whitelist urls with. + * @returns {boolean|ng.$sanitizeProvider} Returns the currently configured value if called + * without an argument or self for chaining otherwise. + */ + this.enableSvg = function(enableSvg) { + if (angular.isDefined(enableSvg)) { + svgEnabled = enableSvg; + return this; + } else { + return svgEnabled; + } + }; } function sanitizeText(chars) { @@ -193,8 +251,7 @@ var validElements = angular.extend({}, voidElements, blockElements, inlineElements, - optionalEndTagElements, - svgElements); + optionalEndTagElements); //Attributes that have href and hence need to be sanitized var uriAttrs = toMap("background,cite,href,longdesc,src,usemap,xlink:href"); diff --git a/test/ngSanitize/filter/linkySpec.js b/test/ngSanitize/filter/linkySpec.js index f0c4f3956e54..11f676a8769e 100644 --- a/test/ngSanitize/filter/linkySpec.js +++ b/test/ngSanitize/filter/linkySpec.js @@ -50,8 +50,10 @@ describe('linky', function() { it('should handle target:', function() { expect(linky("http://example.com", "_blank")). - toEqual('http://example.com'); + toBeOneOf('http://example.com', + 'http://example.com'); expect(linky("http://example.com", "someNamedIFrame")). - toEqual('http://example.com'); + toBeOneOf('http://example.com', + 'http://example.com'); }); }); diff --git a/test/ngSanitize/sanitizeSpec.js b/test/ngSanitize/sanitizeSpec.js index 0f25caa53229..e6bda530dddc 100644 --- a/test/ngSanitize/sanitizeSpec.js +++ b/test/ngSanitize/sanitizeSpec.js @@ -100,7 +100,8 @@ describe('HTML', function() { // THESE TESTS ARE EXECUTED WITH COMPILED ANGULAR it('should echo html', function() { expectHTML('helloworld.'). - toEqual('helloworld.'); + toBeOneOf('helloworld.', + 'helloworld.'); }); it('should remove script', function() { @@ -180,7 +181,8 @@ describe('HTML', function() { it('should ignore back slash as escape', function() { expectHTML('xxx\\'). - toEqual('xxx\\'); + toBeOneOf('xxx\\', + 'xxx\\'); }); it('should ignore object attributes', function() { @@ -214,42 +216,64 @@ describe('HTML', function() { expectHTML(false).toBe('false'); }); - it('should accept SVG tags', function() { - expectHTML('') - .toEqual(''); + it('should strip svg elements if not enabled via provider', function() { + expectHTML('') + .toEqual(''); }); - it('should not ignore white-listed svg camelCased attributes', function() { - expectHTML('') + + describe('SVG support', function() { + + beforeEach(module(function($sanitizeProvider) { + $sanitizeProvider.enableSvg(true); + })); + + + it('should accept SVG tags', function() { + expectHTML('') + .toBeOneOf('', + '', + ''); + }); + + it('should not ignore white-listed svg camelCased attributes', function() { + expectHTML('') .toEqual(''); - }); + }); - it('should sanitize SVG xlink:href attribute values', function() { - expectHTML('') - .toEqual(''); + it('should sanitize SVG xlink:href attribute values', function() { + expectHTML('') + .toBeOneOf('', + ''); - expectHTML('') - .toEqual(''); - }); + expectHTML('') + .toBeOneOf('', + ''); + }); - it('should sanitize unknown namespaced SVG attributes', function() { - expectHTML('') - .toEqual(''); + it('should sanitize unknown namespaced SVG attributes', function() { + expectHTML('') + .toBeOneOf('', + ''); - expectHTML('') - .toEqual(''); - }); + expectHTML('') + .toBeOneOf('', + ''); + }); - it('should not accept SVG animation tags', function() { - expectHTML('Click me') - .toEqual('Click me'); + it('should not accept SVG animation tags', function() { + expectHTML('Click me') + .toEqual('Click me'); - expectHTML('' + - '') - .toEqual(''); + expectHTML('' + + '') + .toBeOneOf('', + ''); + }); }); + describe('htmlSanitizerWriter', function() { /* global htmlSanitizeWriter: false */ if (angular.isUndefined(window.htmlSanitizeWriter)) return;