Skip to content

Commit 9274acf

Browse files
chirayukchirayuk
committed
sec(http): JSONP should require trusted resource URLs
WORK-IN-PROGRESS. Do **NOT** merge. More work needs to be done and the tests are currently broken. - JSONP should require trusted resource URLs. This would be a breaking change but maybe not too onerous since same origin URLs are trusted in the default config and you can easily whitelist any 3rd party URLs you trust in one single place (your app/module config.) - fix a bug where $http can't handle $sce wrapper URLs. Closes angular#11352 Closes angular#11328
1 parent 1459be1 commit 9274acf

File tree

4 files changed

+58
-24
lines changed

4 files changed

+58
-24
lines changed

‎src/ng/http.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,11 @@ function $HttpProvider() {
11101110
cache,
11111111
cachedResp,
11121112
reqHeaders = config.headers,
1113-
url = buildUrl(config.url, config.paramSerializer(config.params));
1113+
// origUrl could be a $sce trusted URL which used a wrapper class
1114+
origUrl = config.url,
1115+
url = $sce.valueOf(origUrl);
1116+
1117+
url = buildUrl(url, config.paramSerializer(config.params));
11141118

11151119
$http.pendingRequests.push(config);
11161120
promise.then(removePendingReq, removePendingReq);
@@ -1154,7 +1158,7 @@ function $HttpProvider() {
11541158
reqHeaders[(config.xsrfHeaderName || defaults.xsrfHeaderName)] = xsrfValue;
11551159
}
11561160

1157-
$httpBackend(config.method, url, reqData, done, reqHeaders, config.timeout,
1161+
$httpBackend(config.method, (url === $sce.valueOf(origUrl) ? origUrl : url), reqData, done, reqHeaders, config.timeout,
11581162
config.withCredentials, config.responseType);
11591163
}
11601164

‎src/ng/httpBackend.js

+10-4
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,20 @@ function createXhr() {
2121
* $httpBackend} which can be trained with responses.
2222
*/
2323
function $HttpBackendProvider() {
24-
this.$get = ['$browser', '$window', '$document', function($browser, $window, $document) {
25-
return createHttpBackend($browser, createXhr, $browser.defer, $window.angular.callbacks, $document[0]);
24+
this.$get = ['$sce', '$browser', '$window', '$document', function($sce, $browser, $window, $document) {
25+
return createHttpBackend($sce, $browser, createXhr, $browser.defer, $window.angular.callbacks, $document[0]);
2626
}];
2727
}
2828

29-
function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDocument) {
29+
function createHttpBackend($sce, $browser, createXhr, $browserDefer, callbacks, rawDocument) {
3030
// TODO(vojta): fix the signature
3131
return function(method, url, post, callback, headers, timeout, withCredentials, responseType) {
32+
if (lowercase(method) == 'jsonp') {
33+
// This is a pretty sensitive operation where we're allowing a script to have full access to
34+
// our DOM and JS space. So we require that the URL satisfies SCE.RESOURCE_URL.
35+
url = $sce.getTrustedResourceUrl(url);
36+
}
37+
3238
$browser.$$incOutstandingRequestCount();
3339
url = url || $browser.url();
3440

@@ -142,8 +148,8 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
142148
// - adds and immediately removes script elements from the document
143149
var script = rawDocument.createElement('script'), callback = null;
144150
script.type = "text/javascript";
145-
script.src = url;
146151
script.async = true;
152+
script.src = url;
147153

148154
callback = function(event) {
149155
removeEventListenerFn(script, "load", callback);

‎src/ngMock/angular-mocks.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,7 @@ angular.mock.dump = function(object) {
10951095
```
10961096
*/
10971097
angular.mock.$HttpBackendProvider = function() {
1098-
this.$get = ['$rootScope', '$timeout', createHttpBackendMock];
1098+
this.$get = ['$sce', '$rootScope', '$timeout', createHttpBackendMock];
10991099
};
11001100

11011101
/**
@@ -1112,7 +1112,7 @@ angular.mock.$HttpBackendProvider = function() {
11121112
* @param {Object=} $browser Auto-flushing enabled if specified
11131113
* @return {Object} Instance of $httpBackend mock
11141114
*/
1115-
function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
1115+
function createHttpBackendMock($sce, $rootScope, $timeout, $delegate, $browser) {
11161116
var definitions = [],
11171117
expectations = [],
11181118
responses = [],
@@ -2092,7 +2092,7 @@ angular.module('ngMockE2E', ['ng']).config(['$provide', function($provide) {
20922092
*/
20932093
angular.mock.e2e = {};
20942094
angular.mock.e2e.$httpBackendDecorator =
2095-
['$rootScope', '$timeout', '$delegate', '$browser', createHttpBackendMock];
2095+
['$sce', '$rootScope', '$timeout', '$delegate', '$browser', createHttpBackendMock];
20962096

20972097

20982098
/**

‎test/ng/httpBackendSpec.js

+39-15
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@
33

44
describe('$httpBackend', function() {
55

6-
var $backend, $browser, callbacks,
6+
var $sce, $backend, $browser, callbacks,
77
xhr, fakeDocument, callback;
88

99

1010
beforeEach(inject(function($injector) {
1111
callbacks = {counter: 0};
12+
$sce = $injector.get('$sce');
1213
$browser = $injector.get('$browser');
1314
fakeDocument = {
1415
$$scripts: [],
@@ -28,7 +29,7 @@ describe('$httpBackend', function() {
2829
})
2930
}
3031
};
31-
$backend = createHttpBackend($browser, createMockXhr, $browser.defer, callbacks, fakeDocument);
32+
$backend = createHttpBackend($sce, $browser, createMockXhr, $browser.defer, callbacks, fakeDocument);
3233
callback = jasmine.createSpy('done');
3334
}));
3435

@@ -255,7 +256,22 @@ describe('$httpBackend', function() {
255256
});
256257

257258

258-
describe('JSONP', function() {
259+
[true, false].forEach(function(trustAsResourceUrl) {
260+
describe('JSONP: trustAsResourceUrl='+trustAsResourceUrl, function() {
261+
262+
263+
function $backendCall() {
264+
var args = Array.prototype.slice.call(arguments);
265+
var url = args[1];
266+
if (trustAsResourceUrl || url == null) {
267+
args[1] = $sce.trustAsResourceUrl(url);
268+
return $backend.apply(null, args);
269+
} else {
270+
var badUrlPrefix = url ? url.replace('JSON_CALLBACK', '') : '';
271+
expect(function() { $backend.apply(null, args); }).toThrowMinErr(
272+
'$sce', 'insecurl', 'Blocked loading resource from url not allowed by $sceDelegate policy. URL: ' + badUrlPrefix);
273+
}
274+
}
259275

260276
var SCRIPT_URL = /([^\?]*)\?cb=angular\.callbacks\.(.*)/;
261277

@@ -266,7 +282,9 @@ describe('$httpBackend', function() {
266282
expect(response).toBe('some-data');
267283
});
268284

269-
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
285+
$backendCall('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
286+
if (!trustAsResourceUrl) return;
287+
270288
expect(fakeDocument.$$scripts.length).toBe(1);
271289

272290
var script = fakeDocument.$$scripts.shift(),
@@ -281,7 +299,9 @@ describe('$httpBackend', function() {
281299

282300

283301
it('should clean up the callback and remove the script', function() {
284-
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
302+
$backendCall('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
303+
if (!trustAsResourceUrl) return;
304+
285305
expect(fakeDocument.$$scripts.length).toBe(1);
286306

287307

@@ -297,11 +317,13 @@ describe('$httpBackend', function() {
297317

298318

299319
it('should set url to current location if not specified or empty string', function() {
300-
$backend('JSONP', undefined, null, callback);
320+
$backendCall('JSONP', undefined, null, callback);
321+
if (!trustAsResourceUrl) return;
322+
301323
expect(fakeDocument.$$scripts[0].src).toBe($browser.url());
302324
fakeDocument.$$scripts.shift();
303325

304-
$backend('JSONP', '', null, callback);
326+
$backendCall('JSONP', '', null, callback);
305327
expect(fakeDocument.$$scripts[0].src).toBe($browser.url());
306328
});
307329

@@ -311,7 +333,9 @@ describe('$httpBackend', function() {
311333
expect(status).toBe(-1);
312334
});
313335

314-
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback, null, 2000);
336+
$backendCall('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback, null, 2000);
337+
if (!trustAsResourceUrl) return;
338+
315339
expect(fakeDocument.$$scripts.length).toBe(1);
316340
expect($browser.deferredFns[0].time).toBe(2000);
317341

@@ -328,7 +352,7 @@ describe('$httpBackend', function() {
328352

329353
// TODO(vojta): test whether it fires "async-start"
330354
// TODO(vojta): test whether it fires "async-end" on both success and error
331-
});
355+
})});
332356

333357
describe('protocols that return 0 status code', function() {
334358

@@ -341,7 +365,7 @@ describe('$httpBackend', function() {
341365

342366

343367
it('should convert 0 to 200 if content and file protocol', function() {
344-
$backend = createHttpBackend($browser, createMockXhr);
368+
$backend = createHttpBackend($sce, $browser, createMockXhr);
345369

346370
$backend('GET', 'file:///whatever/index.html', null, callback);
347371
respond(0, 'SOME CONTENT');
@@ -351,7 +375,7 @@ describe('$httpBackend', function() {
351375
});
352376

353377
it('should convert 0 to 200 if content for protocols other than file', function() {
354-
$backend = createHttpBackend($browser, createMockXhr);
378+
$backend = createHttpBackend($sce, $browser, createMockXhr);
355379

356380
$backend('GET', 'someProtocol:///whatever/index.html', null, callback);
357381
respond(0, 'SOME CONTENT');
@@ -361,7 +385,7 @@ describe('$httpBackend', function() {
361385
});
362386

363387
it('should convert 0 to 404 if no content and file protocol', function() {
364-
$backend = createHttpBackend($browser, createMockXhr);
388+
$backend = createHttpBackend($sce, $browser, createMockXhr);
365389

366390
$backend('GET', 'file:///whatever/index.html', null, callback);
367391
respond(0, '');
@@ -371,7 +395,7 @@ describe('$httpBackend', function() {
371395
});
372396

373397
it('should not convert 0 to 404 if no content for protocols other than file', function() {
374-
$backend = createHttpBackend($browser, createMockXhr);
398+
$backend = createHttpBackend($sce, $browser, createMockXhr);
375399

376400
$backend('GET', 'someProtocol:///whatever/index.html', null, callback);
377401
respond(0, '');
@@ -399,7 +423,7 @@ describe('$httpBackend', function() {
399423

400424
try {
401425

402-
$backend = createHttpBackend($browser, createMockXhr);
426+
$backend = createHttpBackend($sce, $browser, createMockXhr);
403427

404428
$backend('GET', '/whatever/index.html', null, callback);
405429
respond(0, '');
@@ -414,7 +438,7 @@ describe('$httpBackend', function() {
414438

415439

416440
it('should return original backend status code if different from 0', function() {
417-
$backend = createHttpBackend($browser, createMockXhr);
441+
$backend = createHttpBackend($sce, $browser, createMockXhr);
418442

419443
// request to http://
420444
$backend('POST', 'http://rest_api/create_whatever', null, callback);

0 commit comments

Comments
 (0)