Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($sanitize): Use same whitelist mechanism as $compile does. #5137

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions angularFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ angularFiles = {
'src/ng/parse.js',
'src/ng/q.js',
'src/ng/rootScope.js',
'src/ng/sanitizeUri.js',
'src/ng/sce.js',
'src/ng/sniffer.js',
'src/ng/timeout.js',
Expand Down
5 changes: 5 additions & 0 deletions src/AngularPublic.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
$ParseProvider,
$RootScopeProvider,
$QProvider,
$$SanitizeUriProvider,
$SceProvider,
$SceDelegateProvider,
$SnifferProvider,
Expand Down Expand Up @@ -136,6 +137,10 @@ function publishExternalAPI(angular){

angularModule('ng', ['ngLocale'], ['$provide',
function ngModule($provide) {
// $$sanitizeUriProvider needs to be before $compileProvider as it is used by it.
$provide.provider({
$$sanitizeUri: $$SanitizeUriProvider
});
$provide.provider('$compile', $CompileProvider).
directive({
a: htmlAnchorDirective,
Expand Down
34 changes: 12 additions & 22 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -493,14 +493,12 @@ var $compileMinErr = minErr('$compile');
*
* @description
*/
$CompileProvider.$inject = ['$provide'];
function $CompileProvider($provide) {
$CompileProvider.$inject = ['$provide', '$$sanitizeUriProvider'];
function $CompileProvider($provide, $$sanitizeUriProvider) {
var hasDirectives = {},
Suffix = 'Directive',
COMMENT_DIRECTIVE_REGEXP = /^\s*directive\:\s*([\d\w\-_]+)\s+(.*)$/,
CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/,
aHrefSanitizationWhitelist = /^\s*(https?|ftp|mailto|tel|file):/,
imgSrcSanitizationWhitelist = /^\s*(https?|ftp|file):|data:image\//;
CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/;

// Ref: http://developers.whatwg.org/webappapis.html#event-handler-idl-attributes
// The assumption is that future DOM event attribute names will begin with
Expand Down Expand Up @@ -584,10 +582,11 @@ function $CompileProvider($provide) {
*/
this.aHrefSanitizationWhitelist = function(regexp) {
if (isDefined(regexp)) {
aHrefSanitizationWhitelist = regexp;
$$sanitizeUriProvider.aHrefSanitizationWhitelist(regexp);
return this;
} else {
return $$sanitizeUriProvider.aHrefSanitizationWhitelist();
}
return aHrefSanitizationWhitelist;
};


Expand All @@ -614,18 +613,18 @@ function $CompileProvider($provide) {
*/
this.imgSrcSanitizationWhitelist = function(regexp) {
if (isDefined(regexp)) {
imgSrcSanitizationWhitelist = regexp;
$$sanitizeUriProvider.imgSrcSanitizationWhitelist(regexp);
return this;
} else {
return $$sanitizeUriProvider.imgSrcSanitizationWhitelist();
}
return imgSrcSanitizationWhitelist;
};


this.$get = [
'$injector', '$interpolate', '$exceptionHandler', '$http', '$templateCache', '$parse',
'$controller', '$rootScope', '$document', '$sce', '$animate',
'$controller', '$rootScope', '$document', '$sce', '$animate', '$$sanitizeUri',
function($injector, $interpolate, $exceptionHandler, $http, $templateCache, $parse,
$controller, $rootScope, $document, $sce, $animate) {
$controller, $rootScope, $document, $sce, $animate, $$sanitizeUri) {

var Attributes = function(element, attr) {
this.$$element = element;
Expand Down Expand Up @@ -730,16 +729,7 @@ function $CompileProvider($provide) {
// sanitize a[href] and img[src] values
if ((nodeName === 'A' && key === 'href') ||
(nodeName === 'IMG' && key === 'src')) {
// NOTE: urlResolve() doesn't support IE < 8 so we don't sanitize for that case.
if (!msie || msie >= 8 ) {
normalizedVal = urlResolve(value).href;
if (normalizedVal !== '') {
if ((key === 'href' && !normalizedVal.match(aHrefSanitizationWhitelist)) ||
(key === 'src' && !normalizedVal.match(imgSrcSanitizationWhitelist))) {
this[key] = value = 'unsafe:' + normalizedVal;
}
}
}
this[key] = value = $$sanitizeUri(value, key === 'src');
}

if (writeAttr !== false) {
Expand Down
74 changes: 74 additions & 0 deletions src/ng/sanitizeUri.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
'use strict';

/**
* @description
* Private service to sanitize uris for links and images. Used by $compile and $sanitize.
*/
function $$SanitizeUriProvider() {
var aHrefSanitizationWhitelist = /^\s*(https?|ftp|mailto|tel|file):/,
imgSrcSanitizationWhitelist = /^\s*(https?|ftp|file):|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;
};


/**
* @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)) {
imgSrcSanitizationWhitelist = regexp;
return this;
}
return imgSrcSanitizationWhitelist;
};

this.$get = function() {
return function sanitizeUri(uri, isImage) {
var regex = isImage ? imgSrcSanitizationWhitelist : aHrefSanitizationWhitelist;
var normalizedVal;
// NOTE: urlResolve() doesn't support IE < 8 so we don't sanitize for that case.
if (!msie || msie >= 8 ) {
normalizedVal = urlResolve(uri).href;
if (normalizedVal !== '' && !normalizedVal.match(regex)) {
return 'unsafe:'+normalizedVal;
}
}
return uri;
};
};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing blank line at the end of the file

44 changes: 28 additions & 16 deletions src/ngSanitize/filter/linky.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

/* global htmlSanitizeWriter: false */
/* global sanitizeText: false */

/**
* @ngdoc filter
Expand Down Expand Up @@ -100,7 +100,7 @@
</doc:scenario>
</doc:example>
*/
angular.module('ngSanitize').filter('linky', function() {
angular.module('ngSanitize').filter('linky', ['$sanitize', function($sanitize) {
var LINKY_URL_REGEXP =
/((ftp|https?):\/\/|(mailto:)?[A-Za-z0-9._%+-]+@)\S*[^\s.;,(){}<>]/,
MAILTO_REGEXP = /^mailto:/;
Expand All @@ -110,28 +110,40 @@ angular.module('ngSanitize').filter('linky', function() {
var match;
var raw = text;
var html = [];
// TODO(vojta): use $sanitize instead
var writer = htmlSanitizeWriter(html);
var url;
var i;
var properties = {};
if (angular.isDefined(target)) {
properties.target = target;
}
while ((match = raw.match(LINKY_URL_REGEXP))) {
// We can not end in these as they are sometimes found at the end of the sentence
url = match[0];
// if we did not match ftp/http/mailto then assume mailto
if (match[2] == match[3]) url = 'mailto:' + url;
i = match.index;
writer.chars(raw.substr(0, i));
properties.href = url;
writer.start('a', properties);
writer.chars(match[0].replace(MAILTO_REGEXP, ''));
writer.end('a');
addText(raw.substr(0, i));
addLink(url, match[0].replace(MAILTO_REGEXP, ''));
raw = raw.substring(i + match[0].length);
}
writer.chars(raw);
return html.join('');
addText(raw);
return $sanitize(html.join(''));

function addText(text) {
if (!text) {
return;
}
html.push(sanitizeText(text));
}

function addLink(url, text) {
html.push('<a ');
if (angular.isDefined(target)) {
html.push('target="');
html.push(target);
html.push('" ');
}
html.push('href="');
html.push(url);
html.push('">');
addText(text);
html.push('</a>');
}
};
});
}]);
46 changes: 36 additions & 10 deletions src/ngSanitize/sanitize.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ var $sanitizeMinErr = angular.$$minErr('$sanitize');
* 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 whitelist is configured using the functions `aHrefSanitizationWhitelist` and
* `imgSrcSanitizationWhitelist` of {@link ng.$compileProvider `$compileProvider`}.
*
* @param {string} html Html input.
* @returns {string} Sanitized html.
Expand Down Expand Up @@ -128,11 +130,24 @@ var $sanitizeMinErr = angular.$$minErr('$sanitize');
</doc:scenario>
</doc:example>
*/
var $sanitize = function(html) {
function $SanitizeProvider() {
this.$get = ['$$sanitizeUri', function($$sanitizeUri) {
return function(html) {
var buf = [];
htmlParser(html, htmlSanitizeWriter(buf, function(uri, isImage) {
return !/^unsafe/.test($$sanitizeUri(uri, isImage));
}));
return buf.join('');
};
}];
}

function sanitizeText(chars) {
var buf = [];
htmlParser(html, htmlSanitizeWriter(buf));
return buf.join('');
};
var writer = htmlSanitizeWriter(buf, angular.noop);
writer.chars(chars);
return buf.join('');
}


// Regular Expressions for parsing tags and attributes
Expand All @@ -145,7 +160,6 @@ var START_TAG_REGEXP =
COMMENT_REGEXP = /<!--(.*?)-->/g,
DOCTYPE_REGEXP = /<!DOCTYPE([^>]*?)>/i,
CDATA_REGEXP = /<!\[CDATA\[(.*?)]]>/g,
URI_REGEXP = /^((ftp|https?):\/\/|mailto:|tel:|#)/i,
// Match everything outside of normal chars and " (quote character)
NON_ALPHANUMERIC_REGEXP = /([^\#-~| |!])/g;

Expand Down Expand Up @@ -353,8 +367,18 @@ function htmlParser( html, handler ) {
*/
var hiddenPre=document.createElement("pre");
function decodeEntities(value) {
hiddenPre.innerHTML=value.replace(/</g,"&lt;");
return hiddenPre.innerText || hiddenPre.textContent || '';
if (!value) {
return '';
}
// Note: IE8 does not preserve spaces at the start/end of innerHTML
var spaceRe = /^(\s*)([\s\S]*?)(\s*)$/;
var parts = spaceRe.exec(value);
parts[0] = '';
if (parts[2]) {
hiddenPre.innerHTML=parts[2].replace(/</g,"&lt;");
parts[2] = hiddenPre.innerText || hiddenPre.textContent;
}
return parts.join('');
}

/**
Expand Down Expand Up @@ -384,7 +408,7 @@ function encodeEntities(value) {
* comment: function(text) {}
* }
*/
function htmlSanitizeWriter(buf){
function htmlSanitizeWriter(buf, uriValidator){
var ignore = false;
var out = angular.bind(buf, buf.push);
return {
Expand All @@ -398,7 +422,9 @@ function htmlSanitizeWriter(buf){
out(tag);
angular.forEach(attrs, function(value, key){
var lkey=angular.lowercase(key);
if (validAttrs[lkey]===true && (uriAttrs[lkey]!==true || value.match(URI_REGEXP))) {
var isImage = (tag === 'img' && lkey === 'src') || (lkey === 'background');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

background attribute is deprecated in html, I don't think that we should support it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or would that be a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be a breaking change, as it was supported before.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

if (validAttrs[lkey] === true &&
(uriAttrs[lkey] !== true || uriValidator(value, isImage))) {
out(' ');
out(key);
out('="');
Expand Down Expand Up @@ -430,4 +456,4 @@ function htmlSanitizeWriter(buf){


// define ngSanitize module and register $sanitize service
angular.module('ngSanitize', []).value('$sanitize', $sanitize);
angular.module('ngSanitize', []).provider('$sanitize', $SanitizeProvider);
Loading