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

Commit cce98ff

Browse files
adobgkalpak
authored andcommitted
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
1 parent b607618 commit cce98ff

File tree

9 files changed

+166
-13
lines changed

9 files changed

+166
-13
lines changed

src/.eslintrc.json

+1
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@
155155
/* urlUtils.js */
156156
"urlResolve": false,
157157
"urlIsSameOrigin": false,
158+
"urlIsSameOriginAsBaseUrl": false,
158159

159160
/* ng/controller.js */
160161
"identifierForController": false,

src/ng/sce.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ function $SceDelegateProvider() {
227227

228228
function matchUrl(matcher, parsedUrl) {
229229
if (matcher === 'self') {
230-
return urlIsSameOrigin(parsedUrl);
230+
return urlIsSameOrigin(parsedUrl) || urlIsSameOriginAsBaseUrl(parsedUrl);
231231
} else {
232232
// definitely a regex. See adjustMatchers()
233233
return !!matcher.exec(parsedUrl.href);

src/ng/urlUtils.js

+61-11
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// service.
99
var urlParsingNode = window.document.createElement('a');
1010
var originUrl = urlResolve(window.location.href);
11+
var baseUrlParsingNode;
1112

1213

1314
/**
@@ -43,16 +44,16 @@ var originUrl = urlResolve(window.location.href);
4344
* @description Normalizes and parses a URL.
4445
* @returns {object} Returns the normalized URL as a dictionary.
4546
*
46-
* | member name | Description |
47-
* |---------------|----------------|
47+
* | member name | Description |
48+
* |---------------|------------------------------------------------------------------------|
4849
* | href | A normalized version of the provided URL if it was not an absolute URL |
49-
* | protocol | The protocol including the trailing colon |
50+
* | protocol | The protocol without the trailing colon |
5051
* | host | The host and port (if the port is non-default) of the normalizedUrl |
5152
* | search | The search params, minus the question mark |
52-
* | hash | The hash string, minus the hash symbol
53-
* | hostname | The hostname
54-
* | port | The port, without ":"
55-
* | pathname | The pathname, beginning with "/"
53+
* | hash | The hash string, minus the hash symbol |
54+
* | hostname | The hostname |
55+
* | port | The port, without ":" |
56+
* | pathname | The pathname, beginning with "/" |
5657
*
5758
*/
5859
function urlResolve(url) {
@@ -68,7 +69,6 @@ function urlResolve(url) {
6869

6970
urlParsingNode.setAttribute('href', href);
7071

71-
// urlParsingNode provides the UrlUtils interface - http://url.spec.whatwg.org/#urlutils
7272
return {
7373
href: urlParsingNode.href,
7474
protocol: urlParsingNode.protocol ? urlParsingNode.protocol.replace(/:$/, '') : '',
@@ -91,7 +91,57 @@ function urlResolve(url) {
9191
* @returns {boolean} Whether the request is for the same origin as the application document.
9292
*/
9393
function urlIsSameOrigin(requestUrl) {
94-
var parsed = (isString(requestUrl)) ? urlResolve(requestUrl) : requestUrl;
95-
return (parsed.protocol === originUrl.protocol &&
96-
parsed.host === originUrl.host);
94+
return urlsAreSameOrigin(requestUrl, originUrl);
95+
}
96+
97+
/**
98+
* Parse a request URL and determine whether it is same-origin as the current document base URL.
99+
*
100+
* Note: The base URL is usually the same as the document location (`location.href`) but can
101+
* be overriden by using the `<base>` tag.
102+
*
103+
* @param {string|object} requestUrl The url of the request as a string that will be resolved
104+
* or a parsed URL object.
105+
* @returns {boolean} Whether the URL is same-origin as the document base URL.
106+
*/
107+
function urlIsSameOriginAsBaseUrl(requestUrl) {
108+
return urlsAreSameOrigin(requestUrl, getBaseUrl());
109+
}
110+
111+
/**
112+
* Determines if two URLs share the same origin.
113+
*
114+
* @param {string|object} url1 First URL to compare as a string or a normalized URL in the form of
115+
* a dictionary object returned by `urlResolve()`.
116+
* @param {string|object} url2 Second URL to compare as a string or a normalized URL in the form of
117+
* a dictionary object returned by `urlResolve()`.
118+
* @return {boolean} True if both URLs have the same origin, and false otherwise.
119+
*/
120+
function urlsAreSameOrigin(url1, url2) {
121+
url1 = (isString(url1)) ? urlResolve(url1) : url1;
122+
url2 = (isString(url2)) ? urlResolve(url2) : url2;
123+
124+
return (url1.protocol === url2.protocol &&
125+
url1.host === url2.host);
126+
}
127+
128+
/**
129+
* Returns the current document base URL.
130+
* @return {string}
131+
*/
132+
function getBaseUrl() {
133+
if (window.document.baseURI) {
134+
return window.document.baseURI;
135+
}
136+
137+
// document.baseURI is available everywhere except IE
138+
if (!baseUrlParsingNode) {
139+
baseUrlParsingNode = window.document.createElement('a');
140+
baseUrlParsingNode.href = '.';
141+
142+
// Work-around for IE bug described in Implementation Notes. The fix in urlResolve() is not
143+
// suitable here because we need to track changes to the base URL.
144+
baseUrlParsingNode = baseUrlParsingNode.cloneNode(false);
145+
}
146+
return baseUrlParsingNode.href;
97147
}

test/.eslintrc.json

+1
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@
148148
/* urlUtils.js */
149149
"urlResolve": false,
150150
"urlIsSameOrigin": false,
151+
"urlIsSameOriginAsBaseUrl": true,
151152

152153
/* karma */
153154
"dump": false,

test/e2e/fixtures/base-tag/index.html

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<!DOCTYPE html>
2+
<html ng-app="test">
3+
<head>
4+
<base href="http://www.example.com/">
5+
<script src="http://localhost:8000/build/angular.js"></script>
6+
<script src="http://localhost:8000/e2e/fixtures/base-tag/script.js"></script>
7+
</head>
8+
<body>
9+
</body>
10+
</html>

test/e2e/fixtures/base-tag/script.js

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
'use strict';
2+
3+
angular.
4+
module('test', []).
5+
run(function($sce) {
6+
window.isTrustedUrl = function(url) {
7+
try {
8+
$sce.getTrustedResourceUrl(url);
9+
} catch (e) {
10+
return false;
11+
}
12+
return true;
13+
};
14+
});

test/e2e/tests/base-tag.spec.js

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
'use strict';
2+
3+
describe('SCE URL policy when base tags are present', function() {
4+
beforeEach(function() {
5+
loadFixture('base-tag');
6+
});
7+
8+
9+
it('allows the page URL (location.href)', function() {
10+
expectToBeTrusted(browser.getLocationAbsUrl(), true);
11+
});
12+
13+
it('blocks off-origin URLs', function() {
14+
expectToBeTrusted('http://evil.com', false);
15+
});
16+
17+
it('allows relative URLs ("/relative")', function() {
18+
expectToBeTrusted('/relative', true);
19+
});
20+
21+
it('allows absolute URLs from the base origin', function() {
22+
expectToBeTrusted('http://www.example.com/path/to/file.html', true);
23+
});
24+
25+
it('tracks changes to the base URL', function() {
26+
browser.executeScript(
27+
'document.getElementsByTagName("base")[0].href = "http://xxx.example.com/";');
28+
expectToBeTrusted('http://xxx.example.com/path/to/file.html', true);
29+
expectToBeTrusted('http://www.example.com/path/to/file.html', false);
30+
});
31+
32+
33+
// Helpers
34+
function expectToBeTrusted(url, isTrusted) {
35+
var urlIsTrusted = browser.executeScript('return isTrustedUrl(arguments[0])', url);
36+
expect(urlIsTrusted).toBe(isTrusted);
37+
}
38+
});

test/ng/sceSpecs.js

+33
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,39 @@ describe('SCE', function() {
464464
'$sce', 'insecurl', 'Blocked loading resource from url not allowed by $sceDelegate policy. URL: foo');
465465
}
466466
));
467+
468+
describe('when the document base URL has changed', function() {
469+
var baseElem;
470+
var cfg = {whitelist: ['self'], blacklist: []};
471+
472+
beforeEach(function() {
473+
baseElem = window.document.createElement('BASE');
474+
baseElem.setAttribute('href', window.location.protocol + '//foo.example.com/path/');
475+
window.document.head.appendChild(baseElem);
476+
});
477+
478+
afterEach(function() {
479+
window.document.head.removeChild(baseElem);
480+
});
481+
482+
483+
it('should allow relative URLs', runTest(cfg, function($sce) {
484+
expect($sce.getTrustedResourceUrl('foo')).toEqual('foo');
485+
}));
486+
487+
it('should allow absolute URLs', runTest(cfg, function($sce) {
488+
expect($sce.getTrustedResourceUrl('//foo.example.com/bar'))
489+
.toEqual('//foo.example.com/bar');
490+
}));
491+
492+
it('should still block some URLs', runTest(cfg, function($sce) {
493+
expect(function() {
494+
$sce.getTrustedResourceUrl('//bad.example.com');
495+
}).toThrowMinErr('$sce', 'insecurl',
496+
'Blocked loading resource from url not allowed by $sceDelegate policy. ' +
497+
'URL: //bad.example.com');
498+
}));
499+
});
467500
});
468501

469502
it('should have blacklist override the whitelist', runTest(

test/ng/urlUtilsSpec.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,17 @@ describe('urlUtils', function() {
2323
});
2424
});
2525

26-
describe('isSameOrigin', function() {
26+
describe('isSameOrigin and urlIsSameOriginAsBaseUrl', function() {
2727
it('should support various combinations of urls - both string and parsed', inject(function($document) {
2828
function expectIsSameOrigin(url, expectedValue) {
2929
expect(urlIsSameOrigin(url)).toBe(expectedValue);
3030
expect(urlIsSameOrigin(urlResolve(url))).toBe(expectedValue);
31+
32+
// urlIsSameOriginAsBaseUrl() should behave the same as urlIsSameOrigin() by default.
33+
// Behavior when there is a non-default base URL or when the base URL changes dynamically
34+
// is tested in the end-to-end tests in e2e/tests/base-tag.spec.js.
35+
expect(urlIsSameOriginAsBaseUrl(url)).toBe(expectedValue);
36+
expect(urlIsSameOriginAsBaseUrl(urlResolve(url))).toBe(expectedValue);
3137
}
3238
expectIsSameOrigin('path', true);
3339
var origin = urlResolve($document[0].location.href);

0 commit comments

Comments
 (0)