fix($sce): consider document base URL in 'self' URL policy

Page authors can use the `<base>` tag in HTML to specify URL to use as a base
when resovling relative URLs. This can cause SCE to reject relative URLs on the
page, because they fail the same-origin test.

To improve compatibility with the `<base>` tag, this commit changes the logic
for matching URLs to the 'self' policy to allow URLs that match the protocol and
domain of the base URL in addition to URLs that match the loading origin.

**Security Note:**
If an attacker can inject a `<base>` tag into the page, they can circumvent SCE
protections. However, injecting a `<base>` tag typically requires the ability to
inject arbitrary HTML into the page, which is a more serious vulnerabilty than
bypassing SCE.

Fixes #15144

Closes #15145
This commit is contained in:
Alex Dobkin
2016-09-15 14:45:47 -07:00
committed by Georgios Kalpakas
parent b607618342
commit cce98ff53a
9 changed files with 166 additions and 13 deletions
+1
View File
@@ -155,6 +155,7 @@
/* urlUtils.js */ /* urlUtils.js */
"urlResolve": false, "urlResolve": false,
"urlIsSameOrigin": false, "urlIsSameOrigin": false,
"urlIsSameOriginAsBaseUrl": false,
/* ng/controller.js */ /* ng/controller.js */
"identifierForController": false, "identifierForController": false,
+1 -1
View File
@@ -227,7 +227,7 @@ function $SceDelegateProvider() {
function matchUrl(matcher, parsedUrl) { function matchUrl(matcher, parsedUrl) {
if (matcher === 'self') { if (matcher === 'self') {
return urlIsSameOrigin(parsedUrl); return urlIsSameOrigin(parsedUrl) || urlIsSameOriginAsBaseUrl(parsedUrl);
} else { } else {
// definitely a regex. See adjustMatchers() // definitely a regex. See adjustMatchers()
return !!matcher.exec(parsedUrl.href); return !!matcher.exec(parsedUrl.href);
+61 -11
View File
@@ -8,6 +8,7 @@
// service. // service.
var urlParsingNode = window.document.createElement('a'); var urlParsingNode = window.document.createElement('a');
var originUrl = urlResolve(window.location.href); var originUrl = urlResolve(window.location.href);
var baseUrlParsingNode;
/** /**
@@ -43,16 +44,16 @@ var originUrl = urlResolve(window.location.href);
* @description Normalizes and parses a URL. * @description Normalizes and parses a URL.
* @returns {object} Returns the normalized URL as a dictionary. * @returns {object} Returns the normalized URL as a dictionary.
* *
* | member name | Description | * | member name | Description |
* |---------------|----------------| * |---------------|------------------------------------------------------------------------|
* | href | A normalized version of the provided URL if it was not an absolute URL | * | href | A normalized version of the provided URL if it was not an absolute URL |
* | protocol | The protocol including the trailing colon | * | protocol | The protocol without the trailing colon |
* | host | The host and port (if the port is non-default) of the normalizedUrl | * | host | The host and port (if the port is non-default) of the normalizedUrl |
* | search | The search params, minus the question mark | * | search | The search params, minus the question mark |
* | hash | The hash string, minus the hash symbol * | hash | The hash string, minus the hash symbol |
* | hostname | The hostname * | hostname | The hostname |
* | port | The port, without ":" * | port | The port, without ":" |
* | pathname | The pathname, beginning with "/" * | pathname | The pathname, beginning with "/" |
* *
*/ */
function urlResolve(url) { function urlResolve(url) {
@@ -68,7 +69,6 @@ function urlResolve(url) {
urlParsingNode.setAttribute('href', href); urlParsingNode.setAttribute('href', href);
// urlParsingNode provides the UrlUtils interface - http://url.spec.whatwg.org/#urlutils
return { return {
href: urlParsingNode.href, href: urlParsingNode.href,
protocol: urlParsingNode.protocol ? urlParsingNode.protocol.replace(/:$/, '') : '', protocol: urlParsingNode.protocol ? urlParsingNode.protocol.replace(/:$/, '') : '',
@@ -91,7 +91,57 @@ function urlResolve(url) {
* @returns {boolean} Whether the request is for the same origin as the application document. * @returns {boolean} Whether the request is for the same origin as the application document.
*/ */
function urlIsSameOrigin(requestUrl) { function urlIsSameOrigin(requestUrl) {
var parsed = (isString(requestUrl)) ? urlResolve(requestUrl) : requestUrl; return urlsAreSameOrigin(requestUrl, originUrl);
return (parsed.protocol === originUrl.protocol && }
parsed.host === originUrl.host);
/**
* Parse a request URL and determine whether it is same-origin as the current document base URL.
*
* Note: The base URL is usually the same as the document location (`location.href`) but can
* be overriden by using the `<base>` tag.
*
* @param {string|object} requestUrl The url of the request as a string that will be resolved
* or a parsed URL object.
* @returns {boolean} Whether the URL is same-origin as the document base URL.
*/
function urlIsSameOriginAsBaseUrl(requestUrl) {
return urlsAreSameOrigin(requestUrl, getBaseUrl());
}
/**
* Determines if two URLs share the same origin.
*
* @param {string|object} url1 First URL to compare as a string or a normalized URL in the form of
* a dictionary object returned by `urlResolve()`.
* @param {string|object} url2 Second URL to compare as a string or a normalized URL in the form of
* a dictionary object returned by `urlResolve()`.
* @return {boolean} True if both URLs have the same origin, and false otherwise.
*/
function urlsAreSameOrigin(url1, url2) {
url1 = (isString(url1)) ? urlResolve(url1) : url1;
url2 = (isString(url2)) ? urlResolve(url2) : url2;
return (url1.protocol === url2.protocol &&
url1.host === url2.host);
}
/**
* Returns the current document base URL.
* @return {string}
*/
function getBaseUrl() {
if (window.document.baseURI) {
return window.document.baseURI;
}
// document.baseURI is available everywhere except IE
if (!baseUrlParsingNode) {
baseUrlParsingNode = window.document.createElement('a');
baseUrlParsingNode.href = '.';
// Work-around for IE bug described in Implementation Notes. The fix in urlResolve() is not
// suitable here because we need to track changes to the base URL.
baseUrlParsingNode = baseUrlParsingNode.cloneNode(false);
}
return baseUrlParsingNode.href;
} }
+1
View File
@@ -148,6 +148,7 @@
/* urlUtils.js */ /* urlUtils.js */
"urlResolve": false, "urlResolve": false,
"urlIsSameOrigin": false, "urlIsSameOrigin": false,
"urlIsSameOriginAsBaseUrl": true,
/* karma */ /* karma */
"dump": false, "dump": false,
+10
View File
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html ng-app="test">
<head>
<base href="http://www.example.com/">
<script src="http://localhost:8000/build/angular.js"></script>
<script src="http://localhost:8000/e2e/fixtures/base-tag/script.js"></script>
</head>
<body>
</body>
</html>
+14
View File
@@ -0,0 +1,14 @@
'use strict';
angular.
module('test', []).
run(function($sce) {
window.isTrustedUrl = function(url) {
try {
$sce.getTrustedResourceUrl(url);
} catch (e) {
return false;
}
return true;
};
});
+38
View File
@@ -0,0 +1,38 @@
'use strict';
describe('SCE URL policy when base tags are present', function() {
beforeEach(function() {
loadFixture('base-tag');
});
it('allows the page URL (location.href)', function() {
expectToBeTrusted(browser.getLocationAbsUrl(), true);
});
it('blocks off-origin URLs', function() {
expectToBeTrusted('http://evil.com', false);
});
it('allows relative URLs ("/relative")', function() {
expectToBeTrusted('/relative', true);
});
it('allows absolute URLs from the base origin', function() {
expectToBeTrusted('http://www.example.com/path/to/file.html', true);
});
it('tracks changes to the base URL', function() {
browser.executeScript(
'document.getElementsByTagName("base")[0].href = "http://xxx.example.com/";');
expectToBeTrusted('http://xxx.example.com/path/to/file.html', true);
expectToBeTrusted('http://www.example.com/path/to/file.html', false);
});
// Helpers
function expectToBeTrusted(url, isTrusted) {
var urlIsTrusted = browser.executeScript('return isTrustedUrl(arguments[0])', url);
expect(urlIsTrusted).toBe(isTrusted);
}
});
+33
View File
@@ -464,6 +464,39 @@ describe('SCE', function() {
'$sce', 'insecurl', 'Blocked loading resource from url not allowed by $sceDelegate policy. URL: foo'); '$sce', 'insecurl', 'Blocked loading resource from url not allowed by $sceDelegate policy. URL: foo');
} }
)); ));
describe('when the document base URL has changed', function() {
var baseElem;
var cfg = {whitelist: ['self'], blacklist: []};
beforeEach(function() {
baseElem = window.document.createElement('BASE');
baseElem.setAttribute('href', window.location.protocol + '//foo.example.com/path/');
window.document.head.appendChild(baseElem);
});
afterEach(function() {
window.document.head.removeChild(baseElem);
});
it('should allow relative URLs', runTest(cfg, function($sce) {
expect($sce.getTrustedResourceUrl('foo')).toEqual('foo');
}));
it('should allow absolute URLs', runTest(cfg, function($sce) {
expect($sce.getTrustedResourceUrl('//foo.example.com/bar'))
.toEqual('//foo.example.com/bar');
}));
it('should still block some URLs', runTest(cfg, function($sce) {
expect(function() {
$sce.getTrustedResourceUrl('//bad.example.com');
}).toThrowMinErr('$sce', 'insecurl',
'Blocked loading resource from url not allowed by $sceDelegate policy. ' +
'URL: //bad.example.com');
}));
});
}); });
it('should have blacklist override the whitelist', runTest( it('should have blacklist override the whitelist', runTest(
+7 -1
View File
@@ -23,11 +23,17 @@ describe('urlUtils', function() {
}); });
}); });
describe('isSameOrigin', function() { describe('isSameOrigin and urlIsSameOriginAsBaseUrl', function() {
it('should support various combinations of urls - both string and parsed', inject(function($document) { it('should support various combinations of urls - both string and parsed', inject(function($document) {
function expectIsSameOrigin(url, expectedValue) { function expectIsSameOrigin(url, expectedValue) {
expect(urlIsSameOrigin(url)).toBe(expectedValue); expect(urlIsSameOrigin(url)).toBe(expectedValue);
expect(urlIsSameOrigin(urlResolve(url))).toBe(expectedValue); expect(urlIsSameOrigin(urlResolve(url))).toBe(expectedValue);
// urlIsSameOriginAsBaseUrl() should behave the same as urlIsSameOrigin() by default.
// Behavior when there is a non-default base URL or when the base URL changes dynamically
// is tested in the end-to-end tests in e2e/tests/base-tag.spec.js.
expect(urlIsSameOriginAsBaseUrl(url)).toBe(expectedValue);
expect(urlIsSameOriginAsBaseUrl(urlResolve(url))).toBe(expectedValue);
} }
expectIsSameOrigin('path', true); expectIsSameOrigin('path', true);
var origin = urlResolve($document[0].location.href); var origin = urlResolve($document[0].location.href);