Skip to content

Commit f73a651

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

File tree

9 files changed

+13
-166
lines changed

9 files changed

+13
-166
lines changed

src/.eslintrc.json

-1
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,6 @@
157157
/* urlUtils.js */
158158
"urlResolve": false,
159159
"urlIsSameOrigin": false,
160-
"urlIsSameOriginAsBaseUrl": false,
161160

162161
/* ng/controller.js */
163162
"identifierForController": false,

src/ng/sce.js

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

221221
function matchUrl(matcher, parsedUrl) {
222222
if (matcher === 'self') {
223-
return urlIsSameOrigin(parsedUrl) || urlIsSameOriginAsBaseUrl(parsedUrl);
223+
return urlIsSameOrigin(parsedUrl);
224224
} else {
225225
// definitely a regex. See adjustMatchers()
226226
return !!matcher.exec(parsedUrl.href);

src/ng/urlUtils.js

+11-61
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) {
@@ -68,6 +67,7 @@ function urlResolve(url) {
6867

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

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

test/.eslintrc.json

-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,6 @@
149149
/* urlUtils.js */
150150
"urlResolve": false,
151151
"urlIsSameOrigin": false,
152-
"urlIsSameOriginAsBaseUrl": true,
153152

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

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

-10
This file was deleted.

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

-14
This file was deleted.

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

-38
This file was deleted.

test/ng/sceSpecs.js

-33
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

+1-7
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)