Skip to content

Commit 72a87ce

Browse files
petebacondarwinpetebacondarwin
committed
fix(http): do not allow encoded callback params in jsonp requests
1 parent 6d997f5 commit 72a87ce

File tree

2 files changed

+77
-18
lines changed

2 files changed

+77
-18
lines changed

‎src/ng/http.js

+28-18
Original file line numberDiff line numberDiff line change
@@ -1102,7 +1102,7 @@ function $HttpProvider() {
11021102
*
11031103
* @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
11041104
* or an object created by a call to `$sce.trustAsResourceUrl(url)`.
1105-
* @param {Object=} config Optional configuration object
1105+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11061106
* @returns {HttpPromise} Future object
11071107
*/
11081108

@@ -1115,7 +1115,7 @@ function $HttpProvider() {
11151115
*
11161116
* @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
11171117
* or an object created by a call to `$sce.trustAsResourceUrl(url)`.
1118-
* @param {Object=} config Optional configuration object
1118+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11191119
* @returns {HttpPromise} Future object
11201120
*/
11211121

@@ -1128,7 +1128,7 @@ function $HttpProvider() {
11281128
*
11291129
* @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
11301130
* or an object created by a call to `$sce.trustAsResourceUrl(url)`.
1131-
* @param {Object=} config Optional configuration object
1131+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11321132
* @returns {HttpPromise} Future object
11331133
*/
11341134

@@ -1145,6 +1145,10 @@ function $HttpProvider() {
11451145
* {@link $sceDelegateProvider#resourceUrlWhitelist `$sceDelegateProvider.resourceUrlWhitelist`} or
11461146
* by explicitly trusting the URL via {@link $sce#trustAsResourceUrl `$sce.trustAsResourceUrl(url)`}.
11471147
*
1148+
* You should avoid generating the URL for the JSONP request from user provided data.
1149+
* Provide additional query parameters via `params` property of the `config` parameter, rather than
1150+
* modifying the URL itself.
1151+
*
11481152
* JSONP requests must specify a callback to be used in the response from the server. This callback
11491153
* is passed as a query parameter in the request. You must specify the name of this parameter by
11501154
* setting the `jsonpCallbackParam` property on the request config object.
@@ -1166,7 +1170,7 @@ function $HttpProvider() {
11661170
*
11671171
* @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
11681172
* or an object created by a call to `$sce.trustAsResourceUrl(url)`.
1169-
* @param {Object=} config Optional configuration object
1173+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11701174
* @returns {HttpPromise} Future object
11711175
*/
11721176
createShortMethods('get', 'delete', 'head', 'jsonp');
@@ -1180,7 +1184,7 @@ function $HttpProvider() {
11801184
*
11811185
* @param {string} url Relative or absolute URL specifying the destination of the request
11821186
* @param {*} data Request content
1183-
* @param {Object=} config Optional configuration object
1187+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11841188
* @returns {HttpPromise} Future object
11851189
*/
11861190

@@ -1193,7 +1197,7 @@ function $HttpProvider() {
11931197
*
11941198
* @param {string} url Relative or absolute URL specifying the destination of the request
11951199
* @param {*} data Request content
1196-
* @param {Object=} config Optional configuration object
1200+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11971201
* @returns {HttpPromise} Future object
11981202
*/
11991203

@@ -1206,7 +1210,7 @@ function $HttpProvider() {
12061210
*
12071211
* @param {string} url Relative or absolute URL specifying the destination of the request
12081212
* @param {*} data Request content
1209-
* @param {Object=} config Optional configuration object
1213+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
12101214
* @returns {HttpPromise} Future object
12111215
*/
12121216
createShortMethodsWithData('post', 'put', 'patch');
@@ -1420,20 +1424,26 @@ function $HttpProvider() {
14201424
return url;
14211425
}
14221426

1423-
function sanitizeJsonpCallbackParam(url, key) {
1424-
if (/[&?][^=]+=JSON_CALLBACK/.test(url)) {
1425-
// Throw if the url already contains a reference to JSON_CALLBACK
1426-
throw $httpMinErr('badjsonp', 'Illegal use of JSON_CALLBACK in url, "{0}"', url);
1427-
}
1428-
1429-
var callbackParamRegex = new RegExp('[&?]' + key + '=');
1430-
if (callbackParamRegex.test(url)) {
1431-
// Throw if the callback param was already provided
1432-
throw $httpMinErr('badjsonp', 'Illegal use of callback param, "{0}", in url, "{1}"', key, url);
1427+
function sanitizeJsonpCallbackParam(url, cbKey) {
1428+
var parts = url.split('?');
1429+
if (parts.length > 2) {
1430+
// Throw if the url contains more than one `?` query indicator
1431+
throw $httpMinErr('badjsonp', 'Illegal use more than one "?", in url, "{1}"', url);
14331432
}
1433+
var params = parseKeyValue(parts[1]);
1434+
forEach(params, function(value, key) {
1435+
if (value === 'JSON_CALLBACK') {
1436+
// Throw if the url already contains a reference to JSON_CALLBACK
1437+
throw $httpMinErr('badjsonp', 'Illegal use of JSON_CALLBACK in url, "{0}"', url);
1438+
}
1439+
if (key === cbKey) {
1440+
// Throw if the callback param was already provided
1441+
throw $httpMinErr('badjsonp', 'Illegal use of callback param, "{0}", in url, "{1}"', cbKey, url);
1442+
}
1443+
});
14341444

14351445
// Add in the JSON_CALLBACK callback param value
1436-
url += ((url.indexOf('?') === -1) ? '?' : '&') + key + '=JSON_CALLBACK';
1446+
url += ((url.indexOf('?') === -1) ? '?' : '&') + cbKey + '=JSON_CALLBACK';
14371447

14381448
return url;
14391449
}

‎test/ng/httpSpec.js

+49
Original file line numberDiff line numberDiff line change
@@ -979,18 +979,38 @@ describe('$http', function() {
979979
$http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), params: {a: 'b'}});
980980
});
981981

982+
it('should error if the URL contains more than one `?` query indicator', function() {
983+
var error;
984+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?a=b?c=d')})
985+
.catch(function(e) { error = e; });
986+
$rootScope.$digest();
987+
expect(error).toEqualMinErr('$http', 'badjsonp');
988+
});
989+
982990
it('should error if the URL contains a JSON_CALLBACK parameter', function() {
983991
var error;
984992
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?callback=JSON_CALLBACK')})
985993
.catch(function(e) { error = e; });
986994
$rootScope.$digest();
987995
expect(error).toEqualMinErr('$http', 'badjsonp');
988996

997+
error = undefined;
998+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?callback=JSON_C%41LLBACK')})
999+
.catch(function(e) { error = e; });
1000+
$rootScope.$digest();
1001+
expect(error).toEqualMinErr('$http', 'badjsonp');
1002+
9891003
error = undefined;
9901004
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?other=JSON_CALLBACK')})
9911005
.catch(function(e) { error = e; });
9921006
$rootScope.$digest();
9931007
expect(error).toEqualMinErr('$http', 'badjsonp');
1008+
1009+
error = undefined;
1010+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?other=JSON_C%41LLBACK')})
1011+
.catch(function(e) { error = e; });
1012+
$rootScope.$digest();
1013+
expect(error).toEqualMinErr('$http', 'badjsonp');
9941014
});
9951015

9961016
it('should error if a param contains a JSON_CALLBACK value', function() {
@@ -1007,18 +1027,47 @@ describe('$http', function() {
10071027
expect(error).toEqualMinErr('$http', 'badjsonp');
10081028
});
10091029

1030+
it('should allow encoded params that look like they contain the value JSON_CALLBACK or the configured callback key', function() {
1031+
var error;
1032+
error = undefined;
1033+
$httpBackend.expect('JSONP', 'http://example.org/path?other=JSON_C%2541LLBACK&callback=JSON_CALLBACK').respond('');
1034+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {other: 'JSON_C%41LLBACK'}})
1035+
.catch(function(e) { error = e; });
1036+
$rootScope.$digest();
1037+
expect(error).toBeUndefined();
1038+
1039+
error = undefined;
1040+
$httpBackend.expect('JSONP', 'http://example.org/path?c%2561llback=evilThing&callback=JSON_CALLBACK').respond('');
1041+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {'c%61llback': 'evilThing'}})
1042+
.catch(function(e) { error = e; });
1043+
$rootScope.$digest();
1044+
expect(error).toBeUndefined();
1045+
});
1046+
10101047
it('should error if there is already a param matching the jsonpCallbackParam key', function() {
10111048
var error;
10121049
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {callback: 'evilThing'}})
10131050
.catch(function(e) { error = e; });
10141051
$rootScope.$digest();
10151052
expect(error).toEqualMinErr('$http', 'badjsonp');
10161053

1054+
error = undefined;
1055+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?c%61llback=evilThing')})
1056+
.catch(function(e) { error = e; });
1057+
$rootScope.$digest();
1058+
expect(error).toEqualMinErr('$http', 'badjsonp');
1059+
10171060
error = undefined;
10181061
$http({ method: 'JSONP', jsonpCallbackParam: 'cb', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {cb: 'evilThing'}})
10191062
.catch(function(e) { error = e; });
10201063
$rootScope.$digest();
10211064
expect(error).toEqualMinErr('$http', 'badjsonp');
1065+
1066+
error = undefined;
1067+
$http({ method: 'JSONP', jsonpCallbackParam: 'cb', url: $sce.trustAsResourceUrl('http://example.org/path?c%62=evilThing')})
1068+
.catch(function(e) { error = e; });
1069+
$rootScope.$digest();
1070+
expect(error).toEqualMinErr('$http', 'badjsonp');
10221071
});
10231072
});
10241073

0 commit comments

Comments
 (0)