Skip to content

Commit 167bd4c

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 #68832 (cherry picked from commit 231eff1)
1 parent 393b84c commit 167bd4c

5 files changed

Lines changed: 58 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
// (undocumented)
3030
JSONP_HEADERS_NOT_SUPPORTED = 2812,
3131
// (undocumented)
32+
JSONP_UNSAFE_URL = 2826,
33+
// (undocumented)
3234
JSONP_WRONG_METHOD = 2810,
3335
// (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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,5 @@ export const enum RuntimeErrorCode {
3838

3939
FETCH_UPLOAD_PROGRESS_NOT_SUPPORTED = 2824,
4040
FETCH_RESPONSE_BODY_TOO_LARGE = 2825,
41+
JSONP_UNSAFE_URL = 2826,
4142
}

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
*
@@ -139,6 +143,10 @@ export class JsonpClientBackend implements HttpBackend {
139143
);
140144
}
141145

146+
if (!this.isAllowedJsonpUrl(req.urlWithParams)) {
147+
throw new RuntimeError(RuntimeErrorCode.JSONP_UNSAFE_URL, ngDevMode && JSONP_ERR_UNSAFE_URL);
148+
}
149+
142150
// Everything else happens inside the Observable boundary.
143151
return new Observable<HttpEvent<any>>((observer: Observer<HttpEvent<any>>) => {
144152
// The first step to make a request is to generate the callback name, and replace the
@@ -282,6 +290,10 @@ export class JsonpClientBackend implements HttpBackend {
282290

283291
foreignDocument.adoptNode(script);
284292
}
293+
294+
private isAllowedJsonpUrl(url: string): boolean {
295+
return /^https?:\/\//i.test(url);
296+
}
285297
}
286298

287299
/**

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)