Skip to content

Commit 255151a

Browse files
SkyZeroZxatscott
authored andcommitted
fix(http): Rejects non-HTTP(S) URLs in JSONP requests
Prevents JSONP requests from using URLs with unsupported protocols for improved security. Fixes angular#68832
1 parent 79e5d5d commit 255151a

5 files changed

Lines changed: 61 additions & 2 deletions

File tree

goldens/public-api/common/http/errors.api.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export const enum RuntimeErrorCode {
2929
// @deprecated (undocumented)
3030
JSONP_HEADERS_NOT_SUPPORTED = 2812,
3131
// @deprecated (undocumented)
32+
JSONP_UNSAFE_URL = 2826,
33+
// @deprecated (undocumented)
3234
JSONP_WRONG_METHOD = 2810,
3335
// @deprecated (undocumented)
3436
JSONP_WRONG_RESPONSE_TYPE = 2811,

modules/playground/src/jsonp/app/jsonp_comp.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ export class JsonpCmp {
2828
people: Person[] = [];
2929

3030
constructor(http: HttpClient) {
31-
http.jsonp('./people.json', 'callback').subscribe((res: unknown) => {
31+
const peopleUrl = new URL('./people.json', window.location.href).toString();
32+
33+
http.jsonp(peopleUrl, 'callback').subscribe((res: unknown) => {
3234
this.people = res as Person[];
3335
});
3436
}

packages/common/http/src/errors.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,8 @@ export const enum RuntimeErrorCode {
4747

4848
FETCH_UPLOAD_PROGRESS_NOT_SUPPORTED = 2824,
4949
FETCH_RESPONSE_BODY_TOO_LARGE = 2825,
50+
/**
51+
* @deprecated 22.1 JSONP is deprecated as it can cause XSS vulnerabilities. Use standard HTTP requests instead. Intent to remove in future versions of Angular.
52+
*/
53+
JSONP_UNSAFE_URL = 2826,
5054
}

packages/common/http/src/jsonp.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ export const JSONP_ERR_WRONG_RESPONSE_TYPE = 'JSONP requests must use Json respo
5555
// headers set
5656
export const JSONP_ERR_HEADERS_NOT_SUPPORTED = 'JSONP requests do not support headers.';
5757

58+
// Error text given when a JSONP request URL is not absolute HTTP(S).
59+
export const JSONP_ERR_UNSAFE_URL =
60+
'JSONP requests only support absolute URLs with HTTP(S) protocols.';
61+
5862
/**
5963
* DI token/abstract type representing a map of JSONP callbacks.
6064
*
@@ -147,6 +151,10 @@ export class JsonpClientBackend implements HttpBackend {
147151
);
148152
}
149153

154+
if (!this.isAllowedJsonpUrl(req.urlWithParams)) {
155+
throw new RuntimeError(RuntimeErrorCode.JSONP_UNSAFE_URL, ngDevMode && JSONP_ERR_UNSAFE_URL);
156+
}
157+
150158
// Everything else happens inside the Observable boundary.
151159
return new Observable<HttpEvent<any>>((observer: Observer<HttpEvent<any>>) => {
152160
// The first step to make a request is to generate the callback name, and replace the
@@ -290,6 +298,10 @@ export class JsonpClientBackend implements HttpBackend {
290298

291299
foreignDocument.adoptNode(script);
292300
}
301+
302+
private isAllowedJsonpUrl(url: string): boolean {
303+
return /^https?:\/\//i.test(url);
304+
}
293305
}
294306

295307
/**

packages/common/http/test/jsonp_spec.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {HttpHeaders} from '../src/headers';
1212
import {
1313
JSONP_ERR_HEADERS_NOT_SUPPORTED,
1414
JSONP_ERR_NO_CALLBACK,
15+
JSONP_ERR_UNSAFE_URL,
1516
JSONP_ERR_WRONG_METHOD,
1617
JSONP_ERR_WRONG_RESPONSE_TYPE,
1718
JsonpCallbackContext,
@@ -25,7 +26,7 @@ import {toArray} from 'rxjs/operators';
2526
import {MockDocument} from './jsonp_mock';
2627

2728
describe('JsonpClientBackend', () => {
28-
const SAMPLE_REQ = new HttpRequest<never>('JSONP', '/test');
29+
const SAMPLE_REQ = new HttpRequest<never>('JSONP', 'https://example.com/test');
2930
let home: any;
3031
let document: MockDocument;
3132
let backend: JsonpClientBackend;
@@ -127,6 +128,44 @@ describe('JsonpClientBackend', () => {
127128
});
128129
});
129130

131+
describe('URL protocols', () => {
132+
it('allows absolute HTTP(S) URLs', () => {
133+
const urls = [
134+
'http://example.com/test',
135+
'https://example.com/test',
136+
'HTTP://example.com/test',
137+
];
138+
139+
for (const url of urls) {
140+
const subscription = backend.handle(SAMPLE_REQ.clone<never>({url})).subscribe();
141+
142+
subscription.unsubscribe();
143+
}
144+
});
145+
146+
it('rejects URLs without absolute HTTP(S) protocols before creating a script element', () => {
147+
const urls = [
148+
'//example.com/test',
149+
'/test',
150+
'test',
151+
'data:text/javascript,alert(1)',
152+
'blob:https://example.com/jsonp',
153+
'javascript:alert(1)',
154+
'file:///tmp/jsonp.js',
155+
'filesystem:https://example.com/temporary/jsonp.js',
156+
'ftp://example.com/jsonp.js',
157+
'custom-scheme://example.com/jsonp.js',
158+
];
159+
160+
for (const url of urls) {
161+
expect(() => backend.handle(SAMPLE_REQ.clone<never>({url}))).toThrowError(
162+
`NG02826: ${JSONP_ERR_UNSAFE_URL}`,
163+
);
164+
expect(document.mock).toBeUndefined();
165+
}
166+
});
167+
});
168+
130169
describe('throws an error', () => {
131170
it('when request method is not JSONP', () =>
132171
expect(() => backend.handle(SAMPLE_REQ.clone<never>({method: 'GET'}))).toThrowError(

0 commit comments

Comments
 (0)