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

fix($sce): Consider document base URL for 'self' URL policy. #15145

Closed
wants to merge 13 commits into from
1 change: 1 addition & 0 deletions src/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@
/* urlUtils.js */
"urlResolve": false,
"urlIsSameOrigin": false,
"urlIsSameOriginAsBaseUrl": false,

/* ng/controller.js */
"identifierForController": false,
Expand Down
2 changes: 1 addition & 1 deletion src/ng/sce.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ function $SceDelegateProvider() {

function matchUrl(matcher, parsedUrl) {
if (matcher === 'self') {
return urlIsSameOrigin(parsedUrl);
return urlIsSameOrigin(parsedUrl) || urlIsSameOriginAsBaseUrl(parsedUrl);
} else {
// definitely a regex. See adjustMatchers()
return !!matcher.exec(parsedUrl.href);
Expand Down
72 changes: 61 additions & 11 deletions src/ng/urlUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// service.
var urlParsingNode = window.document.createElement('a');
var originUrl = urlResolve(window.location.href);
var baseUrlParsingNode;


/**
Expand Down Expand Up @@ -43,16 +44,16 @@ var originUrl = urlResolve(window.location.href);
* @description Normalizes and parses a URL.
* @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 |
* | 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 |
* | search | The search params, minus the question mark |
* | hash | The hash string, minus the hash symbol
* | hostname | The hostname
* | port | The port, without ":"
* | pathname | The pathname, beginning with "/"
* | hash | The hash string, minus the hash symbol |
* | hostname | The hostname |
* | port | The port, without ":" |
* | pathname | The pathname, beginning with "/" |
*
*/
function urlResolve(url) {
Expand All @@ -68,7 +69,6 @@ function urlResolve(url) {

urlParsingNode.setAttribute('href', href);

// urlParsingNode provides the UrlUtils interface - http://url.spec.whatwg.org/#urlutils
return {
href: urlParsingNode.href,
protocol: urlParsingNode.protocol ? urlParsingNode.protocol.replace(/:$/, '') : '',
Expand All @@ -91,7 +91,57 @@ function urlResolve(url) {
* @returns {boolean} Whether the request is for the same origin as the application document.
*/
function urlIsSameOrigin(requestUrl) {
var parsed = (isString(requestUrl)) ? urlResolve(requestUrl) : requestUrl;
return (parsed.protocol === originUrl.protocol &&
parsed.host === originUrl.host);
return urlsAreSameOrigin(requestUrl, originUrl);
}

/**
* 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) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I would rather not duplicate the code. Maybe have a base function that urlIsSameOrigin/urlIsSameOriginAsBase can delegate two (and pass the originUrl/baseUrl as parameters).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

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 change: 1 addition & 0 deletions test/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@
/* urlUtils.js */
"urlResolve": false,
"urlIsSameOrigin": false,
"urlIsSameOriginAsBaseUrl": true,

/* karma */
"dump": false,
Expand Down
10 changes: 10 additions & 0 deletions test/e2e/fixtures/base-tag/index.html
Original file line number Diff line number Diff line change
@@ -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 changes: 14 additions & 0 deletions test/e2e/fixtures/base-tag/script.js
Original file line number Diff line number Diff line change
@@ -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;
};
});
35 changes: 35 additions & 0 deletions test/e2e/tests/base-tag.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
'use strict';

describe('SCE URL policy when base tags are present', function() {
function checkUrl(url, allowed) {
var urlIsTrusted = browser.executeScript('return isTrustedUrl(arguments[0])', url);
expect(urlIsTrusted).toBe(allowed);
}

beforeAll(function() {
loadFixture('base-tag');
});

it('allows the page URL (location.href)', function() {
checkUrl(browser.getLocationAbsUrl(), true);
});

it('blocks off-origin URLs', function() {
checkUrl('http://evil.com', false);
});

it('allows relative URLs ("/relative")', function() {
checkUrl('/relative', true);
});

it('allows absolute URLs from the base origin', function() {
checkUrl('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/";');
checkUrl('http://xxx.example.com/path/to/file.html', true);
checkUrl('http://www.example.com/path/to/file.html', false);
});
});
26 changes: 26 additions & 0 deletions test/ng/sceSpecs.js
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,32 @@ describe('SCE', function() {
'$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(
Expand Down
8 changes: 7 additions & 1 deletion test/ng/urlUtilsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
function expectIsSameOrigin(url, expectedValue) {
expect(urlIsSameOrigin(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);
var origin = urlResolve($document[0].location.href);
Expand Down