From 0ae184fc05407f44bd581ed3948f7be372c467a4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:16:06 +0000 Subject: [PATCH 1/5] http: fix use-after-free when proxy tunnel closes after response completes mid-read When the final response bytes and the peer's TLS close_notify arrive in one TCP batch, SSLWrapper::handle_reading sets sent_ssl_shutdown before flushing the decrypted bytes through the data callback. If that callback completes the HTTP response, the done path calls ProxyTunnel::shutdown() to tear the wrapper down, but SSLWrapper::shutdown()'s already-shut-down early return skipped trigger_close_callback, leaving closed_notified unset. The result callback then freed the AsyncHTTP embedding the HTTPClient, and handle_reading's subsequent trigger_close_callback invoked on_close on the freed handlers.ctx pointer. Make the fast_shutdown early return fire trigger_close_callback (a no-op when already notified) so the wrapper is marked closed before the owner detaches and frees the ctx. Graceful shutdown(false) is unchanged so half-open TLS reads keep working. --- src/uws/lib.rs | 17 ++++- test/js/bun/http/proxy.test.ts | 121 ++++++++++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/src/uws/lib.rs b/src/uws/lib.rs index 37be10ccad8..8095618962d 100644 --- a/src/uws/lib.rs +++ b/src/uws/lib.rs @@ -565,7 +565,22 @@ pub mod ssl_wrapper { }; // we already sent the ssl shutdown if Self::r(this).flags.sent_ssl_shutdown() || Self::r(this).flags.fatal_error() { - return Self::r(this).flags.received_ssl_shutdown(); + let received = Self::r(this).flags.received_ssl_shutdown(); + if fast_shutdown { + // A fast shutdown is a full teardown. The SSL-level + // shutdown already happened (e.g. the peer's close_notify + // was processed mid handle_reading, which sets + // sent_ssl_shutdown before flushing the final decrypted + // bytes), but closed_notified may not be set yet. The + // owner calls this right before detaching/freeing + // handlers.ctx, so mark the wrapper closed now; otherwise + // handle_reading's deferred trigger_close_callback fires + // on_close into the freed ctx. Idempotent via + // closed_notified; read `received` first because the + // callback may re-enter and tear down state. + Self::r(this).trigger_close_callback(); + } + return received; } // Calling SSL_shutdown() only closes the write direction of the diff --git a/test/js/bun/http/proxy.test.ts b/test/js/bun/http/proxy.test.ts index 96478976974..b2805d7af90 100644 --- a/test/js/bun/http/proxy.test.ts +++ b/test/js/bun/http/proxy.test.ts @@ -1,7 +1,7 @@ import axios from "axios"; import type { Server } from "bun"; import { afterAll, beforeAll, describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, tls as tlsCert } from "harness"; +import { bunEnv, bunExe, isASAN, tls as tlsCert } from "harness"; import { HttpsProxyAgent } from "https-proxy-agent"; import { once } from "node:events"; import net from "node:net"; @@ -908,6 +908,125 @@ test("HTTPS origin close-delimited body via HTTP proxy does not ECONNRESET", asy } }); +// Use-after-free in the proxy tunnel close path: when the final response +// bytes and the TLS close_notify arrive in one TCP batch, SSLWrapper's +// handle_reading sets sent_ssl_shutdown before flushing the decrypted bytes. +// The data callback completes the response, and the done path's +// ProxyTunnel.shutdown() hit SSLWrapper.shutdown()'s already-shut-down early +// return without marking the wrapper closed_notified, so after the client was +// freed handle_reading still fired on_close into the stale handlers.ctx. +// Only deterministic under ASAN; release builds read freed-but-intact memory. +test.skipIf(!isASAN)( + "response + close_notify in one packet via HTTPS proxy tunnel does not use-after-free the client", + async () => { + const fixture = ` + const net = require("node:net"); + const tlsCert = ${JSON.stringify({ cert: tlsCert.cert, key: tlsCert.key })}; + + // HTTPS origin: fixed-length response, then immediate end() so the + // close_notify alert directly follows the application-data record. + const origin = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + tls: tlsCert, + socket: { + data(socket) { + socket.write("HTTP/1.1 200 OK\\r\\nContent-Length: 5\\r\\nConnection: close\\r\\n\\r\\nhello"); + socket.end(); + }, + error() {}, + }, + }); + + // CONNECT proxy that coalesces the tail of the tunnel: after the + // client's second post-CONNECT flight (TLS 1.3 Finished + HTTP request; + // no HelloRetryRequest happens between two BoringSSL peers), + // origin->client bytes are held. The origin finishes its stream with + // close_notify, a tiny alert record (~19 bytes of payload vs 79+ for + // the response/session-ticket records), so once the held byte stream + // ends on a complete alert-sized record everything (tickets + response + // + close_notify) is delivered to the client in ONE write, making the + // fetch client process last-data-then-EOF in a single onData pump. + const proxy = net.createServer(client => { + let upstream = null; + let clientFlights = 0; + let head = Buffer.alloc(0); + let held = Buffer.alloc(0); + const tryFlush = () => { + let o = 0; + let lastIsAlertSized = false; + while (o + 5 <= held.length) { + const len = held.readUInt16BE(o + 3); + if (o + 5 + len > held.length) return; // incomplete record + lastIsAlertSized = len <= 40; + o += 5 + len; + } + if (o !== held.length || !lastIsAlertSized) return; + client.write(held); + held = Buffer.alloc(0); + client.end(); + }; + client.on("error", () => {}); + client.on("close", () => upstream?.destroy()); + client.on("data", chunk => { + if (!upstream) { + head = Buffer.concat([head, chunk]); + const end = head.indexOf("\\r\\n\\r\\n"); + if (end === -1) return; + const leftover = head.subarray(end + 4); + upstream = net.connect(origin.port, "127.0.0.1", () => { + client.write("HTTP/1.1 200 Connection Established\\r\\n\\r\\n"); + if (leftover.length) upstream.write(leftover); + }); + upstream.on("error", () => {}); + upstream.on("data", data => { + if (clientFlights >= 2) { + held = Buffer.concat([held, data]); + tryFlush(); + } else { + client.write(data); + } + }); + return; + } + clientFlights++; + upstream.write(chunk); + }); + }); + + proxy.listen(0, "127.0.0.1", async () => { + const res = await fetch("https://localhost:" + origin.port + "/", { + proxy: "http://127.0.0.1:" + proxy.address().port, + keepalive: false, + tls: { ca: tlsCert.cert, rejectUnauthorized: false }, + }); + const text = await res.text(); + console.log(text); + process.exit(0); + }); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: { + ...bunEnv, + // the explicit per-request proxy must not be bypassed or rerouted by + // ambient proxy configuration on CI hosts + NO_PROXY: undefined, + no_proxy: undefined, + HTTP_PROXY: undefined, + http_proxy: undefined, + HTTPS_PROXY: undefined, + https_proxy: undefined, + }, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toBe("hello\n"); + expect(exitCode).toBe(0); + }, +); + describe.concurrent("proxy object format with headers", () => { test("proxy object with url string works same as string proxy", async () => { const response = await fetch(httpServer.url, { From f0537f20b543f560d121ba4be95dd25ebd674413 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 7 Jun 2026 18:28:00 +0000 Subject: [PATCH 2/5] test: print fixture stderr on failure so the ASAN report is not discarded --- test/js/bun/http/proxy.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/js/bun/http/proxy.test.ts b/test/js/bun/http/proxy.test.ts index b2805d7af90..ab26618c1ff 100644 --- a/test/js/bun/http/proxy.test.ts +++ b/test/js/bun/http/proxy.test.ts @@ -1022,6 +1022,7 @@ test.skipIf(!isASAN)( stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + if (exitCode !== 0) console.error("stderr:", stderr); expect(stdout).toBe("hello\n"); expect(exitCode).toBe(0); }, From 814789de4fee1625f5822b2e75c37cd864421f44 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 00:49:02 +0000 Subject: [PATCH 3/5] test: surface inner-process stderr when the isolate napi regression aborts at teardown The --isolate and --parallel subprocesses in 30205.test.ts can abort after printing their summary line, in which case the toContain assertions pass and only the exit code differs; the captured stderr holding the actual abort report was never shown. Print it on nonzero exit so CI failures are diagnosable. --- test/regression/issue/30205.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/regression/issue/30205.test.ts b/test/regression/issue/30205.test.ts index 9bd23e922fb..406d999a0f1 100644 --- a/test/regression/issue/30205.test.ts +++ b/test/regression/issue/30205.test.ts @@ -99,6 +99,11 @@ describe("#30205", () => { stderr: "pipe", }); const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // A teardown-time abort happens AFTER the summary line, so the + // toContain assertions below still pass and only the exit code + // differs; print the captured stderr so the actual abort report + // (ASAN / JSC assertion) is visible in CI output. + if (exitCode !== 0) console.error("stderr:", stderr); // On crash the summary line is never reached; assert on it (and the pass // count) first so the diff is the actual crash output, not just "expected // 0, got 134". @@ -122,6 +127,7 @@ describe("#30205", () => { stderr: "pipe", }); const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + if (exitCode !== 0) console.error("stderr:", stderr); expect(stderr).toContain("8 pass"); expect(stderr).toContain("0 fail"); expect(stderr).toContain("Ran 8 tests across 8 files."); From 7146b690b389b188891deb2c023567c5079c525f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 18:39:11 +0000 Subject: [PATCH 4/5] jsc: drain cleanup hooks in global_exit so deferred napi finalizers don't leak bun test's exit paths set is_shutting_down directly and call global_exit() without on_exit(), so RareData::cleanup_hooks was never drained and has_run_cleanup_hooks stayed false. NapiFinalizerTask:: schedule() parks finalizers deferred during destructOnExit's final collectNow() on that list when is_shutting_down && !has_run_cleanup_hooks, relying on a drain that never came, leaking every parked Box (LeakSanitizer failures on the x64-asan lane running test/regression/issue/30205.test.ts, tracked in #32144). Drain the hooks at the top of global_exit when on_exit() has not run: parked hooks execute while the heap is still alive, and finalizers enqueued by the final GC take the existing drop path instead of parking. The bun run path is unchanged (on_exit() already ran, so the drain no-ops). --- src/jsc/VirtualMachine.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 99b9524070f..11df4d61017 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -1482,6 +1482,14 @@ impl VirtualMachine { ExitHandler::dispatch_on_exit(self); self.is_shutting_down = true; + self.run_cleanup_hooks(); + } + + /// Drain `RareData::cleanup_hooks` and mark them drained. After this + /// point the list is never iterated again; `NapiFinalizerTask::schedule` + /// keys off `has_run_cleanup_hooks` to drop (instead of park) finalizers + /// deferred during the final `collectNow()`. + fn run_cleanup_hooks(&mut self) { // Make sure we run new cleanup hooks introduced by running cleanup // hooks. // Note: each iteration re-fetches `rare_data` so the FFI hook @@ -1509,6 +1517,19 @@ impl VirtualMachine { // doing it causes like 50+ tests to break // self.event_loop().tick(); + // The test runner's exit paths set `is_shutting_down` directly and + // come here without `on_exit()` (bun test dispatches no process + // 'exit'), so `RareData::cleanup_hooks` was never drained. + // `NapiFinalizerTask::schedule` parks deferred finalizers there while + // `is_shutting_down && !has_run_cleanup_hooks`, relying on a later + // drain that would never happen, leaking every parked + // `Box`. Drain now, while the heap is alive, and + // mark drained so finalizers enqueued by `destructOnExit`'s final + // `collectNow()` below take the drop path instead of parking. + if !self.has_run_cleanup_hooks { + self.run_cleanup_hooks(); + } + if self.should_destruct_main_thread_on_exit() { if let Some(t) = self.event_loop_mut().forever_timer.take() { // SAFETY: `t` is the live usockets timer created in From 1d17b3970457a2f3778ab1dbd2ac1a02ce5d0516 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 19:18:50 +0000 Subject: [PATCH 5/5] Revert "jsc: drain cleanup hooks in global_exit so deferred napi finalizers don't leak" This reverts commit e59bc1d0e0e363f7e723e71bcad8c941d8da2b48. --- src/jsc/VirtualMachine.rs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 11df4d61017..99b9524070f 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -1482,14 +1482,6 @@ impl VirtualMachine { ExitHandler::dispatch_on_exit(self); self.is_shutting_down = true; - self.run_cleanup_hooks(); - } - - /// Drain `RareData::cleanup_hooks` and mark them drained. After this - /// point the list is never iterated again; `NapiFinalizerTask::schedule` - /// keys off `has_run_cleanup_hooks` to drop (instead of park) finalizers - /// deferred during the final `collectNow()`. - fn run_cleanup_hooks(&mut self) { // Make sure we run new cleanup hooks introduced by running cleanup // hooks. // Note: each iteration re-fetches `rare_data` so the FFI hook @@ -1517,19 +1509,6 @@ impl VirtualMachine { // doing it causes like 50+ tests to break // self.event_loop().tick(); - // The test runner's exit paths set `is_shutting_down` directly and - // come here without `on_exit()` (bun test dispatches no process - // 'exit'), so `RareData::cleanup_hooks` was never drained. - // `NapiFinalizerTask::schedule` parks deferred finalizers there while - // `is_shutting_down && !has_run_cleanup_hooks`, relying on a later - // drain that would never happen, leaking every parked - // `Box`. Drain now, while the heap is alive, and - // mark drained so finalizers enqueued by `destructOnExit`'s final - // `collectNow()` below take the drop path instead of parking. - if !self.has_run_cleanup_hooks { - self.run_cleanup_hooks(); - } - if self.should_destruct_main_thread_on_exit() { if let Some(t) = self.event_loop_mut().forever_timer.take() { // SAFETY: `t` is the live usockets timer created in