diff --git a/src/http/ProxyTunnel.rs b/src/http/ProxyTunnel.rs index 5f8a09fc415a..62faf52ea3b1 100644 --- a/src/http/ProxyTunnel.rs +++ b/src/http/ProxyTunnel.rs @@ -697,6 +697,16 @@ impl ProxyTunnel { if let Some(wrapper) = &mut self.wrapper { // fast shutdown the connection let _ = wrapper.shutdown(true); + // This is the completion/fail/redirect teardown (`close_proxy_tunnel`): + // the owning client is detaching from the tunnel and is freed by its + // result callback, or already delivered its result via `fail()`. Mark + // the close notified so a pending `handle_reading` close callback + // self-bails instead of running `on_close` on the freed client when a + // final response and the TLS close_notify arrive in one read. The error + // path (`close_raw`) deliberately does NOT route through here: it relies + // on `on_close` -> `close_and_fail` to deliver the error, so its close + // callback must still fire. + wrapper.mark_close_notified(); } } diff --git a/src/uws/lib.rs b/src/uws/lib.rs index 37be10ccad81..a68cb08818cd 100644 --- a/src/uws/lib.rs +++ b/src/uws/lib.rs @@ -672,6 +672,15 @@ pub mod ssl_wrapper { self.flags.received_ssl_shutdown() && self.flags.sent_ssl_shutdown() } + /// Mark the close as already notified WITHOUT invoking `on_close`. The + /// owner uses this when it is detaching for good and must not receive a + /// further close callback (e.g. the request completed and freed its + /// context). A pending `trigger_close_callback` then no-ops instead of + /// calling `on_close` on a detached or freed handler context. + pub fn mark_close_notified(&self) { + self.flags.set_closed_notified(true); + } + pub fn is_authorized(&self) -> bool { // handshake ended we know if we are authorized or not if self.flags.handshake_state() == HandshakeState::HandshakeCompleted { diff --git a/test/js/web/fetch/fetch-proxy-tunnel-close-uaf-fixture.ts b/test/js/web/fetch/fetch-proxy-tunnel-close-uaf-fixture.ts new file mode 100644 index 000000000000..c6970dad1d8b --- /dev/null +++ b/test/js/web/fetch/fetch-proxy-tunnel-close-uaf-fixture.ts @@ -0,0 +1,115 @@ +// Fixture for fetch-proxy-tunnel-close-uaf.test.ts. Run as a subprocess so the +// parent can strip NO_PROXY/HTTP_PROXY from the environment (loopback proxies +// are otherwise bypassed). +// +// Drives proxied https fetches whose final response bytes and TLS close_notify +// arrive in a single read on Bun's tunnel socket. That makes the inner-TLS +// SSL_read loop return the body, then SSL_ERROR_ZERO_RETURN, inside one +// SSLWrapper::handle_reading: the body flush completes the request and frees the +// HTTPClient, and the close callback that follows in the same dispatch used to +// deref the freed client (heap-use-after-free in ProxyTunnel::on_close). +// +// UAF_MODE=ok : valid Content-Length body; every fetch must resolve to "ok". +// UAF_MODE=malformed: a broken chunked body coalesced with close_notify; every +// fetch must REJECT (the error teardown still has to run, so +// the error is not swallowed, does not hang, and does not UAF). +// +// The upstream sends the response then a TLS close_notify, cork-coalesced into +// one write. The CONNECT proxy forwards every chunk as a single write, so the +// coalesced body+close_notify reach Bun in a single recv. +import net from "node:net"; +import tls from "node:tls"; +import { once } from "node:events"; + +const cert = process.env.UAF_CERT!; +const key = process.env.UAF_KEY!; +const ITERS = Number(process.env.UAF_ITERS ?? 30); +const MODE = process.env.UAF_MODE ?? "ok"; + +// Prove every iteration actually traversed the CONNECT tunnel: `connects` counts +// proxy -> upstream tunnels opened, `served` counts requests the upstream +// received. The target is loopback-reachable, so without these a regression that +// bypasses the `proxy:` option (direct connection) would produce the same +// resolved/rejected split without ever touching ProxyTunnel. +let connects = 0; +let served = 0; + +const upstream = tls.createServer({ key, cert }, sock => { + sock.once("data", () => { + served++; + const res = + MODE === "malformed" + ? // Invalid chunk size ("zz" is not hex) -> chunked decode error. + "HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\nzz\r\n" + : "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok"; + sock.cork(); + sock.write(res); + sock.end(); // close_notify right behind the response record + process.nextTick(() => sock.uncork()); + }); + sock.on("error", () => {}); +}); +await once(upstream.listen(0, "127.0.0.1"), "listening"); +const upPort = (upstream.address() as net.AddressInfo).port; + +const proxy = net.createServer(client => { + // Accumulate until the full CONNECT request line + headers arrive; TCP is a + // stream, so the request can span reads under load. + let head = ""; + const onHead = (buf: Buffer) => { + head += buf.toString("latin1"); + if (!head.includes("\r\n\r\n")) return; + client.off("data", onHead); + const m = /^CONNECT\s+([^:]+):(\d+)/.exec(head); + if (!m) return client.destroy(); + const up = net.connect(Number(m[2]), m[1], () => { + connects++; + client.write("HTTP/1.1 200 Connection Established\r\n\r\n"); + client.on("data", d => up.write(d)); + // Forward each upstream chunk as a single write. The upstream cork-coalesces + // the response and its TLS close_notify into one chunk, so that chunk reaches + // Bun's tunnel socket in one recv (body, then SSL_ERROR_ZERO_RETURN, in one + // handle_reading). Do not wait for upstream 'end': TLS graceful close + // half-waits for Bun's close_notify, which never comes (Bun completes on the + // response itself), so end the client when the upstream socket closes. + up.on("data", d => client.write(d)); + up.on("end", () => client.end()); + up.on("close", () => client.end()); + up.on("error", () => client.destroy()); + }); + up.on("error", () => client.destroy()); + }; + client.on("data", onHead); + client.on("error", () => {}); +}); +await once(proxy.listen(0, "127.0.0.1"), "listening"); +const proxyUrl = `http://127.0.0.1:${(proxy.address() as net.AddressInfo).port}`; + +let resolved = 0; +let rejected = 0; +for (let i = 0; i < ITERS; i++) { + try { + const r = await fetch(`https://127.0.0.1:${upPort}/`, { + proxy: proxyUrl, + tls: { rejectUnauthorized: false }, + keepalive: false, + }); + if ((await r.text()) === "ok") resolved++; + } catch { + rejected++; + } +} + +upstream.close(); +proxy.close(); +// Print all four counts so the parent asserts the full invariant: connects and +// served == ITERS prove every iteration traversed the tunnel and reached the +// upstream (not a direct connection), and the resolved/rejected split is exact +// (resolved=N/rejected=0 for ok, resolved=0/rejected=N for malformed). A +// proxy/TLS/setup failure shows up as the wrong counts rather than masquerading +// as the expected outcome, a swallowed error (hang) never reaches this line, and +// a heap-use-after-free aborts the process under ASan before it prints. +console.log( + `PROXY_TUNNEL_CLOSE_UAF connects=${connects} served=${served} resolved=${resolved} rejected=${rejected} of ${ITERS}`, +); +process.exit(0); diff --git a/test/js/web/fetch/fetch-proxy-tunnel-close-uaf.test.ts b/test/js/web/fetch/fetch-proxy-tunnel-close-uaf.test.ts new file mode 100644 index 000000000000..2859a9c74ebe --- /dev/null +++ b/test/js/web/fetch/fetch-proxy-tunnel-close-uaf.test.ts @@ -0,0 +1,76 @@ +// Regression test: heap-use-after-free in the proxied-TLS read dispatch. +// +// When fetch() goes through an HTTP CONNECT proxy to an `https://` target and a +// response's final body byte and the TLS close_notify arrive in a single read, +// the inner-TLS `SSL_read` loop returns the body, then `SSL_ERROR_ZERO_RETURN`, +// inside one `SSLWrapper::handle_reading`. The body flush completes the request +// and frees the `HTTPClient`; the close callback that follows in the same +// dispatch then dereferenced the freed client (`ProxyTunnel::on_close`, +// src/http/ProxyTunnel.rs). The guard between the two callbacks only checked +// SSLWrapper state, not client liveness, and the completion's +// `wrapper.shutdown(true)` early-returned without setting `closed_notified`. +// +// The fixture forces the body+close_notify coalescing the bug needs and runs +// several sequential proxied fetches; under ASAN the use-after-free aborts the +// subprocess before it prints its success marker. +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tls as tlsCert } from "harness"; + +async function runFixture(mode: "ok" | "malformed") { + await using proc = Bun.spawn({ + cmd: [bunExe(), import.meta.dir + "/fetch-proxy-tunnel-close-uaf-fixture.ts"], + env: (() => { + // Strip proxy env so the explicit loopback `proxy:` option is honored + // (NO_PROXY commonly covers 127.0.0.1, which would bypass the tunnel). + const e: Record = { ...bunEnv }; + for (const k of ["NO_PROXY", "no_proxy", "HTTP_PROXY", "http_proxy", "HTTPS_PROXY", "https_proxy"]) { + delete e[k]; + } + e.UAF_CERT = tlsCert.cert; + e.UAF_KEY = tlsCert.key; + e.UAF_ITERS = "30"; + e.UAF_MODE = mode; + return e; + })(), + stdout: "pipe", + stderr: "pipe", + timeout: 30_000, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + if (exitCode !== 0) console.error(`[${mode}] stderr:`, stderr, "stdout:", stdout); + return { stdout, stderr, exitCode }; +} + +// A heap-use-after-free aborts the subprocess under ASAN; assert the markers are +// absent so an abort can never hide behind stdout handling. +function expectNoAsanAbort(stderr: string) { + expect(stderr).not.toContain("AddressSanitizer"); + expect(stderr).not.toContain("heap-use-after-free"); + expect(stderr).not.toContain("ProxyTunnel"); +} + +// Before the fix the subprocess aborts with a heap-use-after-free in +// ProxyTunnel::on_close (typically within the first couple of iterations) and +// never prints the marker. Assert the exact resolved/rejected split so a +// proxy/TLS/setup failure cannot pass as the expected outcome. stdout is asserted +// before the exit code for a useful failure message. +test("fetch through a CONNECT proxy does not use-after-free on a coalesced response+close_notify", async () => { + const { stdout, stderr, exitCode } = await runFixture("ok"); + expect(stdout).toContain("PROXY_TUNNEL_CLOSE_UAF connects=30 served=30 resolved=30 rejected=0 of 30"); + expectNoAsanAbort(stderr); + expect(exitCode).toBe(0); +}); + +// A malformed response coalesced with close_notify must be delivered as a +// rejection, not swallowed/hung/UAF'd. This single-read variant errors in the +// ProxyHeaders stage (handle_on_data_headers -> fail), so it guards error +// delivery on a coalesced read but does not itself drive the body-stage +// close_from_callback -> close_raw teardown; that path needs two separated reads +// (no deterministic JS signal for the BodyChunk transition) and is verified +// out-of-test via the v1-vs-v2 differential noted in the PR. +test("a malformed proxied response coalesced with close_notify still rejects", async () => { + const { stdout, stderr, exitCode } = await runFixture("malformed"); + expect(stdout).toContain("PROXY_TUNNEL_CLOSE_UAF connects=30 served=30 resolved=0 rejected=30 of 30"); + expectNoAsanAbort(stderr); + expect(exitCode).toBe(0); +});