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

Commit f418ffd

Browse files
committed
revert: fix($sce): consider document base URL in 'self' URL policy
This reverts commit cce98ff. Reverting while investigating security implications of cce98ff without #15597 (which is possibly a breaking change, thus not suitable for this branch).
1 parent 6ab5f8c commit f418ffd

File tree

9 files changed

+13
-166
lines changed

9 files changed

+13
-166
lines changed

src/.eslintrc.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@
155155
/* urlUtils.js */
156156
"urlResolve": false,
157157
"urlIsSameOrigin": false,
158-
"urlIsSameOriginAsBaseUrl": false,
159158

160159
/* ng/controller.js */
161160
"identifierForController": false,

src/ng/sce.js

Lines changed: 1 addition & 1 deletion
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) || urlIsSameOriginAsBaseUrl(parsedUrl);
230+
return urlIsSameOrigin(parsedUrl);
231231
} else {
232232
// definitely a regex. See adjustMatchers()
233233
return !!matcher.exec(parsedUrl.href);

src/ng/urlUtils.js

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

1312

1413
/**
@@ -44,16 +43,16 @@ var baseUrlParsingNode;
4443
* @description Normalizes and parses a URL.
4544
* @returns {object} Returns the normalized URL as a dictionary.
4645
*
47-
* | member name | Description |
48-
* |---------------|------------------------------------------------------------------------|
46+
* | member name | Description |
47+
* |---------------|----------------|
4948
* | href | A normalized version of the provided URL if it was not an absolute URL |
50-
* | protocol | The protocol without the trailing colon |
49+
* | protocol | The protocol including the trailing colon |
5150
* | host | The host and port (if the port is non-default) of the normalizedUrl |
5251
* | search | The search params, minus the question mark |
53-
* | hash | The hash string, minus the hash symbol |
54-
* | hostname | The hostname |
55-
* | port | The port, without ":" |
56-
* | pathname | The pathname, beginning with "/" |
52+
* | hash | The hash string, minus the hash symbol
53+
* | hostname | The hostname
54+
* | port | The port, without ":"
55+
* | pathname | The pathname, beginning with "/"
5756
*
5857
*/
5958
function urlResolve(url) {
@@ -69,6 +68,7 @@ function urlResolve(url) {
6968

7069
urlParsingNode.setAttribute('href', href);
7170

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,57 +91,7 @@ function urlResolve(url) {
9191
* @returns {boolean} Whether the request is for the same origin as the application document.
9292
*/
9393
function urlIsSameOrigin(requestUrl) {
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;
94+
var parsed = (isString(requestUrl)) ? urlResolve(requestUrl) : requestUrl;
95+
return (parsed.protocol === originUrl.protocol &&
96+
parsed.host === originUrl.host);
14797
}

test/.eslintrc.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,6 @@
148148
/* urlUtils.js */
149149
"urlResolve": false,
150150
"urlIsSameOrigin": false,
151-
"urlIsSameOriginAsBaseUrl": true,
152151

153152
/* karma */
154153
"dump": false,

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

Lines changed: 0 additions & 10 deletions
This file was deleted.

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

Lines changed: 0 additions & 14 deletions
This file was deleted.

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

Lines changed: 0 additions & 38 deletions
This file was deleted.

test/ng/sceSpecs.js

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -464,39 +464,6 @@ 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-
});
500467
});
501468

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

test/ng/urlUtilsSpec.js

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

26-
describe('isSameOrigin and urlIsSameOriginAsBaseUrl', function() {
26+
describe('isSameOrigin', 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);
3731
}
3832
expectIsSameOrigin('path', true);
3933
var origin = urlResolve($document[0].location.href);

0 commit comments

Comments
 (0)