diff --git a/.changeset/wise-seas-press.md b/.changeset/wise-seas-press.md new file mode 100644 index 000000000000..b62fe35f90b1 --- /dev/null +++ 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 index a30da78cbade..2c926d60e54d 100644 --- 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 index c09d8e50d845..041b49615eb4 100644 --- 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 = { 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 index 6e29c5efbcff..381fe3e0a836 100644 --- 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(); + 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 index d07625b4cf25..f68bfb367478 100644 --- 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 index 366fba8ee53f..abcc554980d3 100644 --- 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");