fix: validate Host/Origin headers in magic proxy and `InspectorPr…

…oxyWorker` (#4550) * Validate `Host` in Miniflare's proxy server * Validate `Host`/`Origin` headers in `InspectorProxyWorker`
cloudflare · Dec 5, 2023 · 63708a9 · 63708a9
1 parent 86c81ff
commit 63708a9
Show file tree

Hide file tree

Showing 6 changed files with 136 additions and 8 deletions.
diff --git a/.changeset/wise-seas-press.md b/.changeset/wise-seas-press.md
@@ -0,0 +1,8 @@
+---
+"miniflare": patch
+"wrangler": patch
+---
+
+fix: validate `Host` and `Orgin` headers where appropriate
+
+`Host` and `Origin` headers are now checked when connecting to the inspector and Miniflare's magic proxy. If these don't match what's expected, the request will fail.
diff --git a/fixtures/dev-env/tests/index.test.ts b/fixtures/dev-env/tests/index.test.ts
@@ -1,4 +1,6 @@
 import assert from "node:assert";
+import events from "node:events";
+import timers from "node:timers/promises";
 import getPort from "get-port";
 import {
 	Miniflare,
@@ -186,14 +188,17 @@ function fakeReloadComplete(
 		liveReload: config.dev?.liveReload,
 	};
 
-	setTimeout(() => {
+	const timeoutPromise = timers.setTimeout(delay).then(() => {
 		devEnv.proxy.onReloadComplete({
 			type: "reloadComplete",
 			config,
 			bundle: fakeBundle,
 			proxyData,
 		});
-	}, delay);
+	});
+	// Add this promise to `fireAndForgetPromises`, ensuring it runs before we
+	// start the next test
+	fireAndForgetPromises.push(timeoutPromise);
 
 	return { config, mfOpts }; // convenience to allow calling and defining new config/mfOpts inline but also store the new objects
 }
@@ -283,6 +288,35 @@ describe("startDevWorker: ProxyController", () => {
 		});
 	});
 
+	test("InspectorProxyWorker rejects unauthorised requests", async () => {
+		const run = await fakeStartUserWorker({
+			script: `
+				export default {
+					fetch() {
+						return new Response();
+					}
+				}
+			`,
+		});
+
+		// Check validates `Host` header
+		ws = new WebSocket(
+			`ws://${run.inspectorProxyWorkerUrl.host}/core:user:${run.config.name}`,
+			{ setHost: false, headers: { Host: "example.com" } }
+		);
+		let openPromise = events.once(ws, "open");
+		await expect(openPromise).rejects.toThrow("Unexpected server response");
+
+		// Check validates `Origin` header
+		ws = new WebSocket(
+			`ws://${run.inspectorProxyWorkerUrl.host}/core:user:${run.config.name}`,
+			{ origin: "https://example.com" }
+		);
+		openPromise = events.once(ws, "open");
+		await expect(openPromise).rejects.toThrow("Unexpected server response");
+		ws.close();
+	});
+
 	test("User worker exception", async () => {
 		const consoleErrorSpy = vi.spyOn(console, "error");
 

diff --git a/packages/miniflare/src/workers/core/proxy.worker.ts b/packages/miniflare/src/workers/core/proxy.worker.ts
@@ -23,6 +23,7 @@ import {
 
 const ENCODER = new TextEncoder();
 const DECODER = new TextDecoder();
+const ALLOWED_HOSTNAMES = ["127.0.0.1", "[::1]", "localhost"];
 
 const WORKERS_PLATFORM_IMPL: PlatformImpl<ReadableStream> = {
 	Blob,
@@ -132,12 +133,27 @@ export class ProxyServer implements DurableObject {
 	}
 
 	async #fetch(request: Request) {
+		// Validate `Host` header
+		const hostHeader = request.headers.get("Host");
+		if (hostHeader == null) return new Response(null, { status: 400 });
+		try {
+			const host = new URL(`http://${hostHeader}`);
+			if (!ALLOWED_HOSTNAMES.includes(host.hostname)) {
+				return new Response(null, { status: 401 });
+			}
+		} catch {
+			return new Response(null, { status: 400 });
+		}
+
 		// Validate secret header to prevent unauthorised access to proxy
 		const secretHex = request.headers.get(CoreHeaders.OP_SECRET);
 		if (secretHex == null) return new Response(null, { status: 401 });
 		const expectedSecret = this.env[CoreBindings.DATA_PROXY_SECRET];
 		const secretBuffer = Buffer.from(secretHex, "hex");
-		if (!crypto.subtle.timingSafeEqual(secretBuffer, expectedSecret)) {
+		if (
+			secretBuffer.byteLength !== expectedSecret.byteLength ||
+			!crypto.subtle.timingSafeEqual(secretBuffer, expectedSecret)
+		) {
 			return new Response(null, { status: 401 });
 		}
 

diff --git a/packages/miniflare/test/plugins/core/proxy/client.spec.ts b/packages/miniflare/test/plugins/core/proxy/client.spec.ts
@@ -1,5 +1,6 @@
 import assert from "assert";
 import { Blob } from "buffer";
+import http from "http";
 import { text } from "stream/consumers";
 import { ReadableStream } from "stream/web";
 import util from "util";
@@ -185,7 +186,7 @@ test("ProxyClient: stack traces don't include internal implementation", async (t
 
 	const mf = new Miniflare({
 		modules: true,
-		script: `export class DurableObject {}    
+		script: `export class DurableObject {}
     export default {
       fetch() { return new Response(null, { status: 404 }); }
     }`,
@@ -270,9 +271,32 @@ test("ProxyClient: can `JSON.stringify()` proxies", async (t) => {
 test("ProxyServer: prevents unauthorised access", async (t) => {
 	const mf = new Miniflare({ script: nullScript });
 	t.teardown(() => mf.dispose());
-
 	const url = await mf.ready;
-	const res = await fetch(url, { headers: { "MF-Op": "GET" } });
+
+	// Check validates `Host` header
+	const statusPromise = new DeferredPromise<number>();
+	const req = http.get(
+		url,
+		{ setHost: false, headers: { "MF-Op": "GET", Host: "localhost" } },
+		(res) => statusPromise.resolve(res.statusCode ?? 0)
+	);
+	req.on("error", (error) => statusPromise.reject(error));
+	t.is(await statusPromise, 401);
+
+	// Check validates `MF-Op-Secret` header
+	let res = await fetch(url, {
+		headers: { "MF-Op": "GET" }, // (missing)
+	});
+	t.is(res.status, 401);
+	await res.arrayBuffer(); // (drain)
+	res = await fetch(url, {
+		headers: { "MF-Op": "GET", "MF-Op-Secret": "aaaa" }, // (too short)
+	});
+	t.is(res.status, 401);
+	await res.arrayBuffer(); // (drain)
+	res = await fetch(url, {
+		headers: { "MF-Op": "GET", "MF-Op-Secret": "a".repeat(32) }, // (wrong)
+	});
 	t.is(res.status, 401);
 	await res.arrayBuffer(); // (drain)
 });
diff --git a/packages/wrangler/src/api/startDevWorker/ProxyController.ts b/packages/wrangler/src/api/startDevWorker/ProxyController.ts
@@ -67,7 +67,8 @@ export class ProxyController extends EventEmitter {
 			workers: [
 				{
 					name: "ProxyWorker",
-					compatibilityFlags: ["nodejs_compat"],
+					// `url_standard` required to parse IPv6 hostnames correctly
+					compatibilityFlags: ["nodejs_compat", "url_standard"],
 					modulesRoot: path.dirname(proxyWorkerPath),
 					modules: [{ type: "ESModule", path: proxyWorkerPath }],
 					durableObjects: {
@@ -96,7 +97,8 @@ export class ProxyController extends EventEmitter {
 				},
 				{
 					name: "InspectorProxyWorker",
-					compatibilityFlags: ["nodejs_compat"],
+					// `url_standard` required to parse IPv6 hostnames correctly
+					compatibilityFlags: ["nodejs_compat", "url_standard"],
 					modulesRoot: path.dirname(inspectorProxyWorkerPath),
 					modules: [{ type: "ESModule", path: inspectorProxyWorkerPath }],
 					durableObjects: {

diff --git a/packages/wrangler/templates/startDevWorker/InspectorProxyWorker.ts b/packages/wrangler/templates/startDevWorker/InspectorProxyWorker.ts
@@ -19,6 +19,16 @@ import {
 	urlFromParts,
 } from "../../src/api/startDevWorker/utils";
 
+const ALLOWED_HOST_HOSTNAMES = ["127.0.0.1", "[::1]", "localhost"];
+const ALLOWED_ORIGIN_HOSTNAMES = [
+	"devtools.devprod.cloudflare.dev",
+	"cloudflare-devtools.pages.dev",
+	/^[a-z0-9]+\.cloudflare-devtools\.pages\.dev$/,
+	"127.0.0.1",
+	"[::1]",
+	"localhost",
+];
+
 interface Env {
 	PROXY_CONTROLLER: Fetcher;
 	PROXY_CONTROLLER_AUTH_SECRET: string;
@@ -451,6 +461,40 @@ export class InspectorProxyWorker implements DurableObject {
 	}
 
 	async handleDevToolsWebSocketUpgradeRequest(req: Request) {
+		// Validate `Host` header
+		let hostHeader = req.headers.get("Host");
+		if (hostHeader == null) return new Response(null, { status: 400 });
+		try {
+			const host = new URL(`http://${hostHeader}`);
+			if (!ALLOWED_HOST_HOSTNAMES.includes(host.hostname)) {
+				return new Response("Disallowed `Host` header", { status: 401 });
+			}
+		} catch {
+			return new Response("Expected `Host` header", { status: 400 });
+		}
+		// Validate `Origin` header
+		let originHeader = req.headers.get("Origin");
+		if (originHeader === null && !req.headers.has("User-Agent")) {
+			// VSCode doesn't send an `Origin` header, but also doesn't send a
+			// `User-Agent` header, so allow an empty origin in this case.
+			originHeader = "http://localhost";
+		}
+		if (originHeader === null) {
+			return new Response("Expected `Origin` header", { status: 400 });
+		}
+		try {
+			const origin = new URL(originHeader);
+			const allowed = ALLOWED_ORIGIN_HOSTNAMES.some((rule) => {
+				if (typeof rule === "string") return origin.hostname === rule;
+				else return rule.test(origin.hostname);
+			});
+			if (!allowed) {
+				return new Response("Disallowed `Origin` header", { status: 401 });
+			}
+		} catch {
+			return new Response("Expected `Origin` header", { status: 400 });
+		}
+
 		// DevTools attempting to connect
 		this.sendDebugLog("DEVTOOLS WEBSOCKET TRYING TO CONNECT");