Skip to content

Commit f34b870

Browse files
yungstersmeta-codesync[bot]
authored andcommitted
RN: Verify Debugger Connection Origins (facebook#55101)
Summary: Pull Request resolved: facebook#55101 Introduces origin verification for requests to establish debugger connections. Changelog: [General][Fixed] - Changed debugger proxy to only permit connections from localhost. Reviewed By: robhogan Differential Revision: D90348550 fbshipit-source-id: 71589a340d16b831bdb87660963677d7e0890078
1 parent e11a2f0 commit f34b870

File tree

5 files changed

+69
-8
lines changed

5 files changed

+69
-8
lines changed

packages/dev-middleware/src/__tests__/InspectorDebuggerUtils.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,17 @@ export class DebuggerAgent {
2222
#ws: ?WebSocket;
2323
#readyPromise: Promise<void>;
2424

25-
constructor(url: string, signal?: AbortSignal, hostHeader?: ?string) {
25+
constructor(
26+
url: string,
27+
signal?: AbortSignal,
28+
headers?: ?{[string]: unknown},
29+
) {
2630
const ws = new WebSocket(url, {
2731
// The mock server uses a self-signed certificate.
2832
rejectUnauthorized: false,
29-
...(hostHeader != null ? {headers: {Host: hostHeader}} : {}),
33+
...(headers != null
34+
? {headers}
35+
: {headers: {Origin: 'http://localhost:8081'}}),
3036
});
3137
this.#ws = ws;
3238
ws.on('message', data => {
@@ -116,9 +122,9 @@ export class DebuggerMock extends DebuggerAgent {
116122
export async function createDebuggerMock(
117123
url: string,
118124
signal: AbortSignal,
119-
hostHeader?: ?string,
125+
headers?: ?{[string]: unknown},
120126
): Promise<DebuggerMock> {
121-
const debuggerMock = new DebuggerMock(url, signal, hostHeader);
127+
const debuggerMock = new DebuggerMock(url, signal, headers);
122128
await debuggerMock.ready();
123129
return debuggerMock;
124130
}

packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,12 @@ export async function createAndConnectTarget(
116116
signal: AbortSignal,
117117
page: PageFromDevice,
118118
{
119-
debuggerHostHeader = null,
119+
debuggerHeaders = null,
120120
deviceId = null,
121121
deviceHostHeader = null,
122122
}: Readonly<{
123-
debuggerHostHeader?: ?string,
123+
debuggerHeaders?: ?{[string]: unknown},
124+
debuggerOriginHeader?: ?string,
124125
deviceId?: ?string,
125126
deviceHostHeader?: ?string,
126127
}> = {},
@@ -151,7 +152,7 @@ export async function createAndConnectTarget(
151152
debugger_ = await createDebuggerMock(
152153
webSocketDebuggerUrl,
153154
signal,
154-
debuggerHostHeader,
155+
debuggerHeaders,
155156
);
156157
await until(() => expect(device.connect).toBeCalled());
157158
} catch (e) {

packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,10 @@ describe.each(['HTTP', 'HTTPS'])(
273273
vm: 'bar-vm',
274274
},
275275
{
276-
debuggerHostHeader: 'localhost:' + serverRef.port,
276+
debuggerHeaders: {
277+
Host: 'localhost:' + serverRef.port,
278+
Origin: 'http://localhost:8081',
279+
},
277280
deviceHostHeader: sourceHost,
278281
},
279282
);

packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,5 +384,41 @@ describe.each(['HTTP', 'HTTPS'])(
384384
debugger2?.close();
385385
}
386386
});
387+
388+
test('debugger connection with invalid origin is rejected', async () => {
389+
const device1 = await createDeviceMock(
390+
`${serverRef.serverBaseWsUrl}/inspector/device?device=device1&name=foo&app=bar`,
391+
autoCleanup.signal,
392+
);
393+
try {
394+
device1.getPages.mockImplementation(() => [
395+
{
396+
app: 'bar-app',
397+
id: 'page1',
398+
title: 'bar-title',
399+
vm: 'bar-vm',
400+
},
401+
]);
402+
403+
let pageList: JsonPagesListResponse = [];
404+
await until(async () => {
405+
pageList = (await fetchJson(
406+
`${serverRef.serverBaseUrl}/json`,
407+
// $FlowFixMe[unclear-type]
408+
): any);
409+
expect(pageList).toHaveLength(1);
410+
});
411+
const [{webSocketDebuggerUrl}] = pageList;
412+
expect(webSocketDebuggerUrl).toBeDefined();
413+
414+
await expect(
415+
createDebuggerMock(webSocketDebuggerUrl, autoCleanup.signal, {
416+
Origin: 'null',
417+
}),
418+
).rejects.toThrow('Unexpected server response: 401');
419+
} finally {
420+
device1.close();
421+
}
422+
});
387423
},
388424
);

packages/dev-middleware/src/inspector-proxy/InspectorProxy.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ import WS from 'ws';
3232

3333
const debug = require('debug')('Metro:InspectorProxy');
3434

35+
const WS_DEBUGGER_ALLOWED_ORIGINS = new Set([
36+
'http://localhost:8081',
37+
'http://127.0.0.1:8081',
38+
]);
39+
3540
const WS_DEVICE_URL = '/inspector/device';
3641
const WS_DEBUGGER_URL = '/inspector/debug';
3742
const PAGES_LIST_JSON_URL = '/json';
@@ -484,7 +489,17 @@ export default class InspectorProxy implements InspectorProxyQueries {
484489
// Don't crash on exceptionally large messages - assume the debugger is
485490
// well-behaved and the device is prepared to handle large messages.
486491
maxPayload: 0,
492+
// Verify the client is from an allowed origin.
493+
// $FlowFixMe[incompatible-type] - `ws` definition is incomplete.
494+
verifyClient: (
495+
info: Readonly<{
496+
origin: string,
497+
secure: boolean,
498+
req: http$IncomingMessage<>,
499+
}>,
500+
) => WS_DEBUGGER_ALLOWED_ORIGINS.has(info.origin),
487501
});
502+
488503
// $FlowFixMe[value-as-type]
489504
wss.on('connection', async (socket: WS, req) => {
490505
const wssTimestamp = Date.now();

0 commit comments

Comments
 (0)