Skip to content

Commit 1d8dea6

Browse files
rigor789rigor789
committed
fix(debug-ios): only accept debugger connections from this machine and DevTools
The proxy listened on every interface with no origin check, while the debugger can run any code in the app. Anything on the network, or any web page, could connect. It now binds the two loopback addresses and rejects browser origins other than DevTools and localhost.
1 parent 259682e commit 1d8dea6

2 files changed

Lines changed: 158 additions & 29 deletions

File tree

‎lib/device-sockets/ios/app-debug-socket-proxy-factory.ts‎

Lines changed: 83 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,29 @@ import { ITempService } from "../../definitions/temp-service";
1010

1111
const REPLACED_CLOSE_REASON = "Replaced by a newer debugger client";
1212

13+
/**
14+
* The debugger can run any code in the app, and any web page can try to open a
15+
* WebSocket to this port. Clients that aren't browsers, like VS Code's debug
16+
* adapter, send no origin.
17+
*/
18+
export function isAllowedDebuggerOrigin(origin?: string): boolean {
19+
if (!origin) {
20+
return true;
21+
}
22+
23+
try {
24+
const { protocol, hostname, origin: urlOrigin } = new URL(origin);
25+
return (
26+
protocol === "devtools:" ||
27+
protocol === "chrome-devtools:" ||
28+
urlOrigin === "https://chrome-devtools-frontend.appspot.com" ||
29+
["localhost", "127.0.0.1", "[::1]"].includes(hostname)
30+
);
31+
} catch {
32+
return false;
33+
}
34+
}
35+
1336
interface IDebugProxySession {
1437
appDebugSocket: net.Socket;
1538
webSocket?: ws;
@@ -182,11 +205,19 @@ export class AppDebugSocketProxyFactory
182205
return currentSession;
183206
};
184207

185-
const server = await this.listenOnAvailablePort(41000, {
208+
const servers = await this.listenOnAvailablePort(41000, {
186209
verifyClient: (
187210
info: any,
188211
callback: (res: boolean, code?: number, message?: string) => void,
189212
) => {
213+
if (!isAllowedDebuggerOrigin(info.origin)) {
214+
this.$logger.warn(
215+
`Rejected a debugger connection from origin '${info.origin}'.`,
216+
);
217+
callback(false, 403, "Origin not allowed");
218+
return;
219+
}
220+
190221
this.$logger.info("Frontend client connected.");
191222

192223
// Not released on "connection": that never fires for a client that
@@ -207,8 +238,9 @@ export class AppDebugSocketProxyFactory
207238
});
208239
},
209240
});
241+
const server = servers[0];
210242
this.deviceWebServers[cacheKey] = server;
211-
server.on("connection", (webSocket, req) => {
243+
const onConnection = (webSocket: ws, req: any) => {
212244
const encoding = "utf16le";
213245

214246
webSocket.on("error", (err) => {
@@ -278,6 +310,12 @@ export class AppDebugSocketProxyFactory
278310
process.exit(0);
279311
}
280312
});
313+
};
314+
for (const listeningServer of servers) {
315+
listeningServer.on("connection", onConnection);
316+
}
317+
server.once("close", () => {
318+
servers.slice(1).forEach((other) => other.close());
281319
});
282320

283321
return server;
@@ -286,36 +324,56 @@ export class AppDebugSocketProxyFactory
286324
private async listenOnAvailablePort(
287325
startPort: number,
288326
options: ws.ServerOptions,
289-
): Promise<ws.Server> {
327+
): Promise<ws.Server[]> {
290328
// The port can still get taken after the check, by another CLI for example.
291329
let port = await this.$net.getAvailablePortInRange(startPort);
292330
while (true) {
293-
const result = await new Promise<ws.Server | NodeJS.ErrnoException>(
294-
(resolve) => {
295-
const server = new ws.Server({ ...options, port });
296-
server.once("listening", () => {
297-
server.removeAllListeners("error");
298-
server.on("error", (err: Error) => {
299-
this.$logger.trace("Error on debugger web socket server", err);
300-
});
301-
resolve(server);
302-
});
303-
server.once("error", resolve);
304-
},
305-
);
306-
307-
if (result instanceof ws.Server) {
308-
return result;
309-
}
310-
311-
if (result.code !== "EADDRINUSE") {
312-
throw result;
331+
const servers: ws.Server[] = [];
332+
try {
333+
// Only reachable from this machine. Clients told to use "localhost"
334+
// may end up on either loopback address.
335+
servers.push(await this.listen("127.0.0.1", port, options));
336+
const ipv6Server = await this.listen("::1", port, options).catch(
337+
(err: NodeJS.ErrnoException): ws.Server => {
338+
if (err.code === "EADDRINUSE") {
339+
throw err;
340+
}
341+
this.$logger.trace("Not listening on ::1", err);
342+
return null;
343+
},
344+
);
345+
if (ipv6Server) {
346+
servers.push(ipv6Server);
347+
}
348+
return servers;
349+
} catch (err) {
350+
servers.forEach((server) => server.close());
351+
if (err.code !== "EADDRINUSE") {
352+
throw err;
353+
}
354+
port = await this.$net.getAvailablePortInRange(port + 1);
313355
}
314-
315-
port = await this.$net.getAvailablePortInRange(port + 1);
316356
}
317357
}
318358

359+
private listen(
360+
host: string,
361+
port: number,
362+
options: ws.ServerOptions,
363+
): Promise<ws.Server> {
364+
return new Promise<ws.Server>((resolve, reject) => {
365+
const server = new ws.Server({ ...options, host, port });
366+
server.once("listening", () => {
367+
server.removeAllListeners("error");
368+
server.on("error", (err: Error) => {
369+
this.$logger.trace("Error on debugger web socket server", err);
370+
});
371+
resolve(server);
372+
});
373+
server.once("error", reject);
374+
});
375+
}
376+
319377
public removeAllProxies() {
320378
let deviceId;
321379
for (deviceId in this.deviceWebServers) {

‎test/device-sockets/app-debug-socket-proxy-factory.ts‎

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import { assert } from "chai";
22
import * as net from "net";
3+
import * as os from "os";
34
import * as ws from "ws";
45
import { MessageUnpackStream } from "ios-device-lib";
5-
import { AppDebugSocketProxyFactory } from "../../lib/device-sockets/ios/app-debug-socket-proxy-factory";
6+
import {
7+
AppDebugSocketProxyFactory,
8+
isAllowedDebuggerOrigin,
9+
} from "../../lib/device-sockets/ios/app-debug-socket-proxy-factory";
610

711
const REPLACED_CLOSE_REASON = "Replaced by a newer debugger client";
812

@@ -87,13 +91,18 @@ describe("AppDebugSocketProxyFactory", () => {
8791
return <number>(<any>server.address()).port;
8892
};
8993

90-
const open = (port: number) =>
94+
const open = (port: number, options?: ws.ClientOptions, host = "127.0.0.1") =>
9195
new Promise<ws>((resolve, reject) => {
92-
const client = new ws(`ws://127.0.0.1:${port}`);
96+
const client = new ws(`ws://${host}:${port}`, options);
9397
client.once("open", () => resolve(client));
9498
client.once("error", reject);
9599
});
96100

101+
const assertRejects = async (promise: Promise<ws>) => {
102+
const client = await promise.catch((): ws => null);
103+
assert.isNull(client);
104+
};
105+
97106
const roundTrip = (client: ws, message: string) =>
98107
new Promise<string>((resolve, reject) => {
99108
client.once("message", (data) => resolve(data.toString()));
@@ -125,7 +134,9 @@ describe("AppDebugSocketProxyFactory", () => {
125134

126135
it("moves to the next port when another server takes the checked one", async () => {
127136
const blocker = net.createServer();
128-
await new Promise<void>((resolve) => blocker.listen(0, () => resolve()));
137+
await new Promise<void>((resolve) =>
138+
blocker.listen(0, "127.0.0.1", () => resolve()),
139+
);
129140
const blockedPort = (<net.AddressInfo>blocker.address()).port;
130141
const ports = [blockedPort];
131142
const factory = createProxy(async () => {
@@ -147,6 +158,34 @@ describe("AppDebugSocketProxyFactory", () => {
147158
assert.equal(await roundTrip(client, "hello"), "hello");
148159
});
149160

161+
it("listens on both loopback addresses and nothing else", async () => {
162+
const port = await startProxy();
163+
164+
for (const host of ["127.0.0.1", "[::1]"]) {
165+
const client = await open(port, undefined, host);
166+
assert.equal(await roundTrip(client, host), host);
167+
}
168+
169+
const addresses = Object.values(os.networkInterfaces())
170+
.flat()
171+
.filter((address) => !address.internal && address.family === "IPv4");
172+
for (const { address } of addresses) {
173+
await assertRejects(open(port, undefined, address));
174+
}
175+
});
176+
177+
it("turns away web pages but not DevTools or clients without an origin", async () => {
178+
const port = await startProxy();
179+
180+
await assertRejects(open(port, { origin: "https://evil.example" }));
181+
assert.equal(backendConnections, 0);
182+
183+
const devtools = await open(port, { origin: "devtools://devtools" });
184+
assert.equal(await roundTrip(devtools, "hello"), "hello");
185+
const noOrigin = await open(port);
186+
assert.equal(await roundTrip(noOrigin, "hello"), "hello");
187+
});
188+
150189
it("serves a client after the previous one disconnected", async () => {
151190
const port = await startProxy();
152191
for (let i = 0; i < 3; i++) {
@@ -216,4 +255,36 @@ describe("AppDebugSocketProxyFactory", () => {
216255
assert.lengthOf(connected, 1);
217256
assert.equal(await roundTrip(connected[0], "hello"), "hello");
218257
});
258+
259+
describe("isAllowedDebuggerOrigin", () => {
260+
const allowed = [
261+
undefined,
262+
"",
263+
"devtools://devtools",
264+
"chrome-devtools://devtools",
265+
"https://chrome-devtools-frontend.appspot.com",
266+
"http://localhost:5173",
267+
"http://127.0.0.1:8080",
268+
"http://[::1]:3000",
269+
];
270+
for (const origin of allowed) {
271+
it(`allows ${origin || "no origin"}`, () => {
272+
assert.isTrue(isAllowedDebuggerOrigin(origin));
273+
});
274+
}
275+
276+
const rejected = [
277+
"https://evil.example",
278+
"http://localhost.evil.example",
279+
"http://127.0.0.1.evil.example",
280+
"https://chrome-devtools-frontend.appspot.com.evil.example",
281+
"null",
282+
"file://",
283+
];
284+
for (const origin of rejected) {
285+
it(`rejects ${origin}`, () => {
286+
assert.isFalse(isAllowedDebuggerOrigin(origin));
287+
});
288+
}
289+
});
219290
});

0 commit comments

Comments
 (0)