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

Commit

Permalink
fix($compile): sanitize values bound to img[src]
Browse files Browse the repository at this point in the history
Ref: 9532234

BREAKING CHANGE: img[src] URLs are now sanitized using the same whitelist
    as a[href] URLs.  The most obvious impact is if you were using data:
    URIs.  data: URIs will be whitelisted for img[src] in a future
    commit.
  • Loading branch information
chirayuk committed Jun 22, 2013
1 parent 99e85fc commit 1adf29a
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 9 deletions.
20 changes: 12 additions & 8 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,15 @@ function $CompileProvider($provide) {
*
* @description
* Retrieves or overrides the default regular expression that is used for whitelisting of safe
* urls during a[href] sanitization.
* urls during a[href] and img[src] 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 `urlSanitizationWhitelist` 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 it is written into the DOM.
* Any url about to be assigned to a[href] or img[src] via data-binding is first normalized and
* turned into an absolute url. Afterwards, the url is matched against the
* `urlSanitizationWhitelist` 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
Expand Down Expand Up @@ -264,7 +265,8 @@ function $CompileProvider($provide) {
$set: function(key, value, writeAttr, attrName) {
var booleanKey = getBooleanAttrName(this.$$element[0], key),
$$observers = this.$$observers,
normalizedVal;
normalizedVal,
nodeName;

if (booleanKey) {
this.$$element.prop(key, value);
Expand All @@ -284,8 +286,10 @@ function $CompileProvider($provide) {
}


// sanitize a[href] values
if (nodeName_(this.$$element[0]) === 'A' && key === 'href') {
// sanitize a[href] and img[src] values
nodeName = nodeName_(this.$$element);
if ((nodeName === 'A' && key === 'href') ||
(nodeName === 'IMG' && key === 'src')){
urlSanitizationNode.setAttribute('href', value);

// href property always returns normalized absolute url, so we can match against that
Expand Down
151 changes: 150 additions & 1 deletion test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2536,7 +2536,156 @@ describe('$compile', function() {
});


describe('href sanitization', function() {
describe('img[src] sanitization', function() {
it('should NOT require trusted values for img src', inject(function($rootScope, $compile) {
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);
$rootScope.testUrl = 'http://example.com/image.png';
$rootScope.$digest();
expect(element.attr('src')).toEqual('http://example.com/image.png');
}));

it('should sanitize javascript: urls', inject(function($compile, $rootScope) {
element = $compile('<img src="{{testUrl}}"></a>')($rootScope);
$rootScope.testUrl = "javascript:doEvilStuff()";
$rootScope.$apply();
expect(element.attr('src')).toBe('unsafe:javascript:doEvilStuff()');
}));

it('should sanitize data: urls', inject(function($compile, $rootScope) {
element = $compile('<img src="{{testUrl}}"></a>')($rootScope);
$rootScope.testUrl = "data:evilPayload";
$rootScope.$apply();

expect(element.attr('src')).toBe('unsafe:data:evilPayload');
}));


it('should sanitize obfuscated javascript: urls', inject(function($compile, $rootScope) {
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);

// case-sensitive
$rootScope.testUrl = "JaVaScRiPt:doEvilStuff()";
$rootScope.$apply();
expect(element[0].src).toBe('unsafe:javascript:doEvilStuff()');

// tab in protocol
$rootScope.testUrl = "java\u0009script:doEvilStuff()";
$rootScope.$apply();
expect(element[0].src).toMatch(/(http:\/\/|unsafe:javascript:doEvilStuff\(\))/);

// space before
$rootScope.testUrl = " javascript:doEvilStuff()";
$rootScope.$apply();
expect(element[0].src).toBe('unsafe:javascript:doEvilStuff()');

// ws chars before
$rootScope.testUrl = " \u000e javascript:doEvilStuff()";
$rootScope.$apply();
expect(element[0].src).toMatch(/(http:\/\/|unsafe:javascript:doEvilStuff\(\))/);

// post-fixed with proper url
$rootScope.testUrl = "javascript:doEvilStuff(); http://make.me/look/good";
$rootScope.$apply();
expect(element[0].src).toBeOneOf(
'unsafe:javascript:doEvilStuff(); http://make.me/look/good',
'unsafe:javascript:doEvilStuff();%20http://make.me/look/good'
);
}));

it('should sanitize ng-src bindings as well', inject(function($compile, $rootScope) {
element = $compile('<img ng-src="{{testUrl}}"></img>')($rootScope);
$rootScope.testUrl = "javascript:doEvilStuff()";
$rootScope.$apply();

expect(element[0].src).toBe('unsafe:javascript:doEvilStuff()');
}));


it('should not sanitize valid urls', inject(function($compile, $rootScope) {
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);

$rootScope.testUrl = "foo/bar";
$rootScope.$apply();
expect(element.attr('src')).toBe('foo/bar');

$rootScope.testUrl = "/foo/bar";
$rootScope.$apply();
expect(element.attr('src')).toBe('/foo/bar');

$rootScope.testUrl = "../foo/bar";
$rootScope.$apply();
expect(element.attr('src')).toBe('../foo/bar');

$rootScope.testUrl = "#foo";
$rootScope.$apply();
expect(element.attr('src')).toBe('#foo');

$rootScope.testUrl = "http://foo/bar";
$rootScope.$apply();
expect(element.attr('src')).toBe('http://foo/bar');

$rootScope.testUrl = " http://foo/bar";
$rootScope.$apply();
expect(element.attr('src')).toBe(' http://foo/bar');

$rootScope.testUrl = "https://foo/bar";
$rootScope.$apply();
expect(element.attr('src')).toBe('https://foo/bar');

$rootScope.testUrl = "ftp://foo/bar";
$rootScope.$apply();
expect(element.attr('src')).toBe('ftp://foo/bar');

// Fails on IE < 10 with "TypeError: Access is denied" when trying to set img[src]
if (!msie || msie > 10) {
$rootScope.testUrl = "mailto:[email protected]";
$rootScope.$apply();
expect(element.attr('src')).toBe('mailto:[email protected]');
}

$rootScope.testUrl = "file:///foo/bar.html";
$rootScope.$apply();
expect(element.attr('src')).toBe('file:///foo/bar.html');
}));


it('should not sanitize attributes other than src', inject(function($compile, $rootScope) {
element = $compile('<img title="{{testUrl}}"></img>')($rootScope);
$rootScope.testUrl = "javascript:doEvilStuff()";
$rootScope.$apply();

expect(element.attr('title')).toBe('javascript:doEvilStuff()');
}));


it('should allow reconfiguration of the src whitelist', function() {
module(function($compileProvider) {
expect($compileProvider.urlSanitizationWhitelist() instanceof RegExp).toBe(true);
var returnVal = $compileProvider.urlSanitizationWhitelist(/javascript:/);
expect(returnVal).toBe($compileProvider);
});

inject(function($compile, $rootScope) {
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);

// Fails on IE < 10 with "TypeError: Object doesn't support this property or method" when
// trying to set img[src]
if (!msie || msie > 10) {
$rootScope.testUrl = "javascript:doEvilStuff()";
$rootScope.$apply();
expect(element.attr('src')).toBe('javascript:doEvilStuff()');
}

$rootScope.testUrl = "http://recon/figured";
$rootScope.$apply();
expect(element.attr('src')).toBe('unsafe:http://recon/figured');
});
});

});


describe('a[href] sanitization', function() {

it('should sanitize javascript: urls', inject(function($compile, $rootScope) {
element = $compile('<a href="{{testUrl}}"></a>')($rootScope);
Expand Down

0 comments on commit 1adf29a

Please sign in to comment.