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

Commit

Permalink
revert: fix($sce): consider document base URL in 'self' URL policy
Browse files Browse the repository at this point in the history
This reverts commit 5e28b6e.
Reverting while investigating security implications of 5e28b6e without #15597
(which is possibly a breaking change, thus not suitable for this branch).
  • Loading branch information
gkalpak committed Jan 12, 2017
1 parent d656369 commit f73a651
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 166 deletions.
1 change: 0 additions & 1 deletion src/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@
/* 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 @@ -220,7 +220,7 @@ function $SceDelegateProvider() {

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


/**
Expand Down Expand Up @@ -44,16 +43,16 @@ var baseUrlParsingNode;
* @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 without the trailing colon |
* | protocol | The protocol including 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,6 +67,7 @@ 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 @@ -90,57 +90,7 @@ function urlResolve(url) {
* @returns {boolean} Whether the request is for the same origin as the application document.
*/
function urlIsSameOrigin(requestUrl) {
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) {
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;
var parsed = (isString(requestUrl)) ? urlResolve(requestUrl) : requestUrl;
return (parsed.protocol === originUrl.protocol &&
parsed.host === originUrl.host);
}
1 change: 0 additions & 1 deletion test/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@
/* urlUtils.js */
"urlResolve": false,
"urlIsSameOrigin": false,
"urlIsSameOriginAsBaseUrl": true,

/* karma */
"dump": false,
Expand Down
10 changes: 0 additions & 10 deletions test/e2e/fixtures/base-tag/index.html

This file was deleted.

14 changes: 0 additions & 14 deletions test/e2e/fixtures/base-tag/script.js

This file was deleted.

38 changes: 0 additions & 38 deletions test/e2e/tests/base-tag.spec.js

This file was deleted.

33 changes: 0 additions & 33 deletions test/ng/sceSpecs.js
Original file line number Diff line number Diff line change
Expand Up @@ -464,39 +464,6 @@ 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: 1 addition & 7 deletions test/ng/urlUtilsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,11 @@ describe('urlUtils', function() {
});
});

describe('isSameOrigin and urlIsSameOriginAsBaseUrl', function() {
describe('isSameOrigin', 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

0 comments on commit f73a651

Please sign in to comment.