http: fix use-after-free when proxy tunnel closes after response completes mid-read#31959
Conversation
|
Updated 1:46 PM PT - Jun 17th, 2026
✅ @cirospaciari, your commit 37d8722a6f3b3ae72b12ca90d91408f5d206d54a passed in 🧪 To try this PR locally: bunx bun-pr 31959That installs a local version of the PR into your bun-31959 --bun |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR ensures SSLWrapper::shutdown invokes the close callback in the fast-shutdown path, adds an ASAN-only regression test that reproduces response bytes coalesced with TLS close_notify, and makes child-process stderr visible on non-zero exits in an existing regression test. ChangesSSL/TLS Shutdown and Callback Invocation
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any bugs and the fix looks well-reasoned, but this touches re-entrant callback ordering in the core SSLWrapper used by ProxyTunnel, UpgradedDuplex, WebSocketProxyTunnel, and WindowsNamedPipe — worth a human pass on the memory-safety invariants.
Extended reasoning...
Overview
The PR fixes a use-after-free in SSLWrapper::shutdown() (src/uws/lib.rs). When fast_shutdown=true hits the already-shut-down early return (sent_ssl_shutdown || fatal_error), it now fires trigger_close_callback() so closed_notified is set before the owner frees handlers.ctx. This closes the gap where handle_reading's deferred close-callback would otherwise fire into a freed HTTPClient. A new ASAN-only regression test in proxy.test.ts constructs a CONNECT proxy that coalesces response + close_notify into one TCP write to deterministically reproduce the UAF.
Security risks
No new attack surface is introduced — this is a defensive fix for a UAF in client-side TLS teardown. However, the change runs inside the BoringSSL wrapper's re-entrant callback machinery, so an incorrect fix here could itself introduce UAF or double-close in a different consumer.
Level of scrutiny
High. SSLWrapper::shutdown(true) has four consumers (HTTP ProxyTunnel, UpgradedDuplex, WebSocketProxyTunnel, WindowsNamedPipe), and the new trigger_close_callback() fires their respective on_close handlers from a code path that previously did not. I traced the ProxyTunnel case (the motivating bug): close_proxy_tunnel does proxy_tunnel.take() before tunnel.shutdown(), so the re-entrant on_close sees proxy_tunnel == None and returns early — only closed_notified is set, which is exactly the intent. The UpgradedDuplex/WebSocket paths look consistent with the existing non-early-return fast-shutdown branch (which already fires the callback unconditionally), and idempotency via closed_notified guards double-fire. But the correctness here hinges on subtle aliasing and ordering invariants documented in scattered comments (LaunderedSelf, disjoint field accessors, NLL-dead borrows), and I'd want a human familiar with this subsystem to confirm none of the other consumers' on_close handlers assume they won't fire from inside their own teardown path.
Other factors
The PR description is thorough (ASAN trace, root-cause walkthrough, verification across 7 test suites), the code change itself is ~15 lines with a detailed inline comment, and received is correctly read before the callback to survive re-entrant teardown. The test is well-constructed (TLS-record-aware coalescing proxy) though ASAN-gated. This is a good PR — it's just not a mechanical change I can rubber-stamp.
|
Audited the fourth consumer the review mentions, WebSocketProxyTunnel (src/http_jsc/websocket_client/WebSocketProxyTunnel.rs):
Same conclusion for UpgradedDuplex (ctx is the duplex that embeds the wrapper) and WindowsNamedPipe (only ever calls |
There was a problem hiding this comment.
Thanks for addressing the stderr nit. The fix itself reads correctly and the consumer audit is convincing, but since this changes callback re-entry timing in shared SSLWrapper::shutdown (TLS lifetime code touching ProxyTunnel/UpgradedDuplex/WebSocketProxyTunnel) and there's a competing approach in #31952, I'd like a human to make the final call.
Extended reasoning...
Overview
The PR adds ~15 lines to SSLWrapper::shutdown() in src/uws/lib.rs: when fast_shutdown=true hits the already-shut-down early return (sent_ssl_shutdown || fatal_error), it now fires trigger_close_callback() so closed_notified is set before the owner frees handlers.ctx. This closes a UAF where handle_reading's deferred on_close fired into a freed HTTPClient after the data callback completed the response and tore down the tunnel mid-read. A new ASAN-gated regression test in proxy.test.ts deterministically reproduces the batched response+close_notify scenario via a record-coalescing CONNECT proxy.
Since my prior review, commit f8b4675 added the if (exitCode !== 0) console.error("stderr:", stderr) line I'd suggested — that nit is resolved.
Security risks
No new attack surface. The change is a memory-safety hardening (UAF fix), not a protocol or auth change. The only behavioral delta is that on_close may now fire synchronously from the early-return branch of a fast shutdown — the author audited all four shutdown(true) callers (ProxyTunnel, UpgradedDuplex, WebSocketProxyTunnel; WindowsNamedPipe only uses shutdown(false)) and showed re-entry is bounded by the closed_notified idempotency guard, matching what the normal fast-shutdown path already does.
Level of scrutiny
High. This is production-critical TLS/HTTP-client code with subtle unsafe lifetime and re-entry semantics (laundered &mut self, handler vtables that can re-enter via the owning socket). The fix is small and the reasoning is well-documented, but the blast radius spans every SSLWrapper consumer, and the "fire vs. suppress the callback" design choice differs from the competing PR #31952. A maintainer familiar with the SSLWrapper consumers should confirm the re-entry analysis and pick between the two approaches.
Other factors
- The bug-hunting system found no issues.
receivedis correctly captured beforetrigger_close_callback()(which may re-enter and mutate flags), preserving the original return value.- The test is ASAN-only by design (release builds read freed-but-intact memory), follows the file's subprocess conventions, and now surfaces the ASAN report on failure.
- No CODEOWNERS entry covers these paths.
|
CI status: the diff is green; all remaining failures in build 61324 are unrelated to this change.
The new regression test (ASAN-gated, in |
…letes 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.
cirospaciari
force-pushed
the
farm/06ff7d9a/fix-proxy-tunnel-close-uaf
branch
from
1fe7c4e to
e7a03c0
Compare
|
@robobun adopt |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/http/proxy.test.ts`:
- Around line 919-1029: This ASAN subprocess test lacks the standard 60_000ms
timeout and can flake under slow ASAN/debug runs; update the test invocation
that starts with test.skipIf(!isASAN)( "response + close_notify...", async () =>
{ ... } ) to pass a 60_000 timeout (i.e. add the 60_000 ms timeout as the third
argument to the test.skipIf(...) call), so the spawned Bun subprocess (Bun.spawn
/ bunExe usage) has enough time to complete under ASAN.
- Around line 1024-1027: The test currently only prints stderr when exitCode !==
0 which can miss ASAN diagnostics; change the assertions in the block around
proc/stdout/stderr/exited to assert stderr first and fail if it contains
sanitizer diagnostics (e.g., check stderr is empty or does not match
/AddressSanitizer|ASAN|ERROR: AddressSanitizer/), then assert stdout and
exitCode; update the variables referenced (proc, stdout, stderr, exitCode) in
the test to make the stderr check the first expectation so sanitizer output
causes the test to fail immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c54676d-7b61-4990-920d-32ff13cc32fa
📒 Files selected for processing (2)
src/uws/lib.rstest/js/bun/http/proxy.test.ts
|
@robobun fix the asan failure |
…borts 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.
|
Investigated the ASAN failure (
The push re-runs CI; if that shard fails again the abort report will now be in the log and I'll dig into it from there. |
|
The stderr surfacing from e20464e did its job: the asan failure is a LeakSanitizer report at process exit, not a use-after-free. The final exit GC sweeps the last fixture file's ~1000 napi-wrapped objects; their deferred finalizer tasks are enqueued into a sink that never drains at that point of teardown, so the boxes are live when LSAN scans. This is pre-existing and independent of this PR's diff (the 30205 fixtures do no networking, so the SSLWrapper change never executes there); it will intermittently fail any PR whose asan shard runs that file. Filed #32144 with the full analysis; happy to take that fix in a separate PR. |
|
@robobun can you fix CI erros? |
…on'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<NapiFinalizerTask> (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).
|
Fixed in e59bc1d. Root cause of the asan failure:
The fix makes On a deterministic regression test: the leak needs the exit-time GC to be the first thing that sweeps the wrappers, which depends on JSC sweep scheduling. I could not trigger that window locally (40+ ASAN runs of the 30205 scenario plus three targeted fixture variants), so the proof is the failing shard itself: builds 61984 and 61990 show the |
…lizers don't leak" This reverts commit e59bc1d.
|
Update on the asan failure: I reverted my cleanup-hook drain (e59bc1d, reverted in 61f9e70). Build 62135 showed why: with the drain in place but nothing skipping pending #32146 is the complete version of this fix and was already in flight: it carries the same |
cirospaciari
force-pushed
the
farm/06ff7d9a/fix-proxy-tunnel-close-uaf
branch
from
61f9e70 to
1d17b39
Compare
…y socket is dead at ProxyHeaders write (#32462) ## Crash signature `Panic: Internal assertion failure` inside `on_writable` after the proxy-tunnel inner TLS handshake completes. Sentry BUN-2V7Z (~1080 lifetime, ~28/24h) and BUN-36JT (sibling). 100% Windows; affected 1.3.x back to firstSeen 2026-04-11. Stack (bun 1.3.14): ``` onData src/http/HTTPContext.zig onData src/http/http.zig receive src/http/ProxyTunnel.zig receiveData / handleTraffic / updateHandshakeState / triggerHandshakeCallback onHandshake src/http/ProxyTunnel.zig onWritable src/http/http.zig <- assert(!socket.isShutdown()); assert(!socket.isClosed()); ``` ## Cause `on_writable` has two sites that asserted the outer socket was live immediately before writing the initial request: * `send_initial_request_payload` (Pending/Headers/Opened arm) * the `ProxyHeaders` arm (proxy tunnel: first HTTP write after the inner TLS handshake) When `fetch()` goes through a CONNECT proxy to an `https://` target, the inner TLS handshake runs inside an in-process `SSLWrapper`. `ProxyTunnel.on_handshake` fires from `handleTraffic` while draining buffered bytes delivered by a single `onData` call, then synchronously calls `HTTPClient.on_writable`. Between that call and the `ProxyHeaders` write, the outer socket can already be closed or shut down: * `proxy.on_writable` (run first, line 2849) writes buffered encrypted bytes with `socket.write()` and then `wrapper.flush()` re-enters `handleTraffic`; on an HTTPS proxy, a failing outer `SSL_write` sets `ssl_fatal_error` so `socket.is_shutdown()` flips true, and a buffered inner `close_notify` fires `on_close -> close_and_fail -> terminate_socket` so `socket.is_closed()` flips true. In release builds without debug assertions, both sites fall through to `write_to_socket` / `ProxyTunnel::write` which return 0 on a dead socket, leaving the request stuck at `Headers` / `ProxyHeaders` until the idle timeout. In builds with debug assertions enabled the assert panicked. The Sentry cluster is 100% Windows because Windows Zig release builds used `ReleaseSafe` (see `scripts/build/zig.ts` at the affected revisions: "since Bun 1.1, Windows builds use ReleaseSafe"), which enables `Environment.allow_assert`. Other platforms shipped `ReleaseFast` where the assert compiled out. Since the Rust rewrite the assert is `debug_assert!`, so release builds on all platforms now stall instead of panicking; `bun bd` still panics. ## Fix Convert both assertion sites to real runtime checks that surface `error.ConnectionClosed` through the existing `close_and_fail` path. `send_initial_request_payload` already returns `Result<_, Error>` and its caller already `close_and_fail`s on error; the `ProxyHeaders` arm calls `close_and_fail` directly (matching the adjacent `write_request` error branch). ## Verification New test in `test/js/bun/http/proxy.test.ts` (fixture at `proxy-handshake-closed-socket-fixture.ts`) runs 10 fetches through an HTTP and HTTPS CONNECT proxy that relays the backend's TLS server-handshake flight and immediately RSTs the outer connection. Every iteration rejects with `ECONNRESET` / `ConnectionClosed` (not `TimeoutError`, not a panic). Full `proxy.test.ts` (47 tests) passes. ### Note on fail-before This race is not deterministically reproducible on Linux loopback. A 100-iteration probe against an unfixed `bun bd` build (50 HTTP-proxy + 50 HTTPS-proxy) never tripped the assertion: on POSIX the proxy's RST is delivered to the client as a separate event loop iteration after `on_writable` has already returned, so `socket.is_closed()/is_shutdown()` is still false at the assertion site. The Windows-specific timing that lands the dead-socket state inside the synchronous `on_handshake -> on_writable` window (AFD poll semantics plus ReleaseSafe assertions) cannot be reconstructed without instrumenting `src/`. The new test guards the code path and the observable behaviour (clean rejection, no stall) but does not fail-before on Linux. ## Related * #31959 fixes a UAF on the *response* side of the same tunnel (`handle_reading` after the client is freed); this PR is the *request* side (first write after handshake). * #23092 is a different crash site in the same function. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…AF through tunnel (#32635) ## What Comprehensive stress testing of the HTTP client proxy code paths (fetch and WebSocket, both `ProxyTunnel` and `WebSocketProxyTunnel`): **661 tests** across 5 new files plus a shared adversarial proxy/origin helper and a subprocess memory-probe fixture. | File | Tests | Covers | | --- | --- | --- | | `proxy-stress-helpers.ts` | n/a | Adversarial CONNECT + absolute-form proxy (http/https outer) with per-stage RST/kill/trickle/split hooks; adversarial origin with content-length / chunked / close-delimited × identity/gzip/deflate/br/zstd, truncation at arbitrary byte offset, redirect, echo. | | `proxy-stress-matrix.test.ts` | 335 | `{http,https}` proxy × `{http,https}` origin × framing × encoding × body-size × keepalive response matrix; upload matrix across string/Uint8Array/Blob/FormData/ReadableStream/async-iterator; streamed response via `getReader()`; trickled 1-byte-per-tick downstream; split CONNECT envelope; RFC 9110 §9.3.6 ignored-header handling; hop-by-hop stripping; method matrix; cross-scheme redirects. | | `proxy-stress-lifecycle.test.ts` | 93 | Proxy RSTs client at every tunnel stage (request-received / upstream-connected / connect-replied / first-client-byte / first-upstream-byte) × both origins × both keepalive; same during upload (string + stream body); proxy drops upstream at every stage; origin RST at 0/10/60/200 response bytes; close-delimited × compression; abort at every stage; abort after headers; abort churn (200× under ASAN); ASAN-only TLS-alert-during-handshake UAF repro. | | `proxy-stress-errors.test.ts` | 51 | CONNECT 400/403/407/500/502/503/504/301/302; proxy unreachable; upstream unreachable (502 via CONNECT and absolute-form); proxy auth (missing/wrong/correct, URL and header) for all 4 combos; inner-TLS verify (CA match, no-CA fail, checkServerIdentity reject); unsupported scheme (ftp/socks4/socks5/socks5h/ws); h2-capable origin through proxy stays HTTP/1.1. | | `proxy-stress-concurrent.test.ts` | 31 | 32× parallel per combo × keepalive; tunnel reuse (sequential + auth-keyed); 12 origins through one proxy; `reject_unauthorized` pool gate (lax → strict forces fresh CONNECT); 4× concurrent 4MB echo; idle pooled tunnel receiving stray data is evicted; 12 subprocess memory probes (6 modes × 2 proxy schemes, 300-1200 iterations each, RSS-growth bounded). | | `proxy-stress-adversarial.test.ts` | 151 | Stacked split-CONNECT × chunked × compression × keepalive; origin status 200-503 × all combos; origin-facing Host/header shape; `checkServerIdentity` approve/reject/GC-loop; path/query edges; `verbose:true`; `AbortSignal.timeout`; interleaved proxy/direct to same origin; CONNECT target shape; **WebSocket proxy matrix** (ws/wss × http/https proxy) with echo, 256KB binary, RST at CONNECT, 407 without auth, rapid close, wss-via-https-proxy open/close churn under GC. | Full suite runs in ~5 min under debug+ASAN. ## Bugs found and fixed Two pre-existing bugs surfaced by the suite (both reproduce on main without any test-helper changes). Fixed here in `src/http/lib.rs`. ### 1. Streamed request body through a CONNECT tunnel never sent `fetch()` to an `https://` origin through any proxy with a `ReadableStream` / async-iterator body hangs forever: CONNECT succeeds, inner TLS handshake completes, the `Transfer-Encoding: chunked` request head is written, then nothing. String/Uint8Array/Blob/FormData bodies are fine; `http://` origins (absolute-form, no tunnel) are fine. Cause (two layers): - The `RequestStage::ProxyHeaders` arm of `on_writable` computes `has_sent_body = self.request_body().is_empty()`. For `HTTPRequestBody::Stream` the bytes buffer is always empty here, so the request jumps straight to `RequestStage::Done` and the `is_streaming_request_body` signal path a few lines below never runs. The non-proxy `send_initial_request_payload` already gates this on `matches!(original_request_body, HTTPRequestBody::Bytes(_))`. - Even once `ProxyBody` is reached, its `Stream` case calls `flush_stream` → `write_to_stream_using_buffer` → `write_to_socket(socket, …)`, which writes plaintext chunked bytes to the outer proxy socket instead of through the inner TLS session. The `Bytes` case right next to it correctly routes through `ProxyTunnel::write`. Fix: mirror the non-proxy path's `Bytes`-only `has_sent_body` check (and matching `debug_assert`) in the `ProxyHeaders` arm; in `write_to_stream_using_buffer`, route through `ProxyTunnel::write` when `self.proxy_tunnel.is_some()`, treating `WantRead`/`WantWrite` from the inner SSL as backpressure. Encrypted output reaches the outer socket via the existing `write_encrypted` callback, same as the `Bytes` path. This also covers the `HTTPThread` drain loop which calls the same `flush_stream`. ### 2. `heap-use-after-free` in `HTTPClient::on_writable` when a TLS alert is buffered with the inner handshake flight ``` READ of size 1 thread T12 (HTTP Client) #0 HTTPClient::on_writable::<true, false> src/http/lib.rs:~2856 #1 bun_http::proxy_tunnel::on_handshake src/http/ProxyTunnel.rs:450 freed by: AsyncHTTP::on_async_http_callback_raw src/http/AsyncHTTP.rs:813 HTTPClient::close_and_fail::<false> bun_http::proxy_tunnel::on_close src/http/ProxyTunnel.rs:577 SSLWrapper::handle_reading src/uws/lib.rs:1053 SSLWrapper::flush src/uws/lib.rs:669 ProxyTunnel::on_writable::<false> src/http/ProxyTunnel.rs:740 HTTPClient::on_writable::<true, false> src/http/lib.rs:~2850 bun_http::proxy_tunnel::on_handshake src/http/ProxyTunnel.rs:450 ``` If the origin closes or sends a TLS alert in the same buffer as its ServerHello flight (origin rejecting client cert, corrupted stream via misbehaving proxy, origin crash), the client's `on_handshake → on_writable → proxy.on_writable → SSLWrapper::flush → handle_reading` chain processes the alert, fires `on_close → close_and_fail`, which runs the result callback and frees the `ThreadlocalAsyncHTTP` embedding `*self`. Control returns to `on_writable`, which immediately reads `self.state.flags.is_waiting_for_cert_check` on freed memory. Same bug class already documented in `start_proxy_handshake`'s comment. Fix: `close_and_fail` → `terminate_socket` synchronously marks the outer socket closed, and the socket handle is owned by the event loop (outlives the client). Check `socket.is_closed()` immediately after `proxy.on_writable()` and return before touching `self`. Deterministic ASAN repro in `proxy-stress-lifecycle.test.ts` ("TLS alert in same buffer as inner handshake"): a CONNECT proxy that double-writes every client→upstream byte, making the origin's TLS stack abort with an alert. Looped 20× with `ASAN_OPTIONS=…:abort_on_error=1` so the HTTP-thread UAF aborts the subprocess before the main thread's clean exit wins the race. ## Verification ``` bun bd test test/js/bun/http/proxy-stress-*.test.ts # 661 pass, 0 fail (debug+ASAN, ~5min) ``` Fail-before (src/ stashed, `bun bd`): - `proxy-stress-matrix.test.ts -t "https-origin POST ReadableStream|https-origin POST async-iterator"` → 8 fail (hang/timeout) - `proxy-stress-lifecycle.test.ts -t "TLS alert in same buffer"` → 1 fail (subprocess aborts with `heap-use-after-free`) Existing `test/js/bun/http/proxy.test.ts` (48 tests) still passes. ## Related - #31959 fixed a sibling UAF on the shutdown side of the same `SSLWrapper` callback chain. - #30606 touches the same `ProxyTunnel` close path but only in the `.zig` reference files. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2 participants
Fixes a use-after-free in the HTTP client's proxy tunnel close path (Sentry BUN-2VY8, ~10 events/day on Windows release builds; reproduces deterministically under ASAN on all platforms).
Repro
fetch()through an HTTP CONNECT proxy to an HTTPS origin, where the origin's final response bytes and its TLSclose_notifyreach the client in a single TCP batch (origin writes the response and immediately closes). The regression test builds exactly that: a local CONNECT proxy that holds origin-to-client bytes after the handshake and flushes session tickets + response + close_notify in one write.On an unfixed ASAN build:
Cause
handle_readingprocesses the batch:SSL_readreturns the body bytes, the nextSSL_readhitsclose_notify(SSL_ERROR_ZERO_RETURN), which setsreceived_ssl_shutdownandsent_ssl_shutdownbefore flushing the already-decrypted bytes through the data callback.close_proxy_tunnel(true)->ProxyTunnel::shutdown()->SSLWrapper::shutdown(true), which hits the already-shut-down early return (sent_ssl_shutdown || fatal_error) and returns without settingclosed_notified. The result callback then frees theThreadlocalAsyncHTTPembedding theHTTPClient, the exact pointer stored in the wrapper'shandlers.ctx.handle_reading. Its liveness guard (ssl.is_none() || closed_notified()) passes because neither is set, sotrigger_close_callback()invokeson_close(handlers.ctx)on the freed client. When the allocation has been recycled,on_closecan ref or close a different request's tunnel instead of faulting.Fix
src/uws/lib.rs: whenSSLWrapper::shutdown(fast_shutdown=true)takes the already-shut-down early return, firetrigger_close_callback()(idempotent viaclosed_notified) so the wrapper is marked closed before the owner detaches and freeshandlers.ctx. A fast shutdown is a full teardown, and the normal fast-shutdown path already fires the close callback unconditionally; this only closes the gap where the SSL-level shutdown had already happened.Graceful
shutdown(false)(node:tls half-close via UpgradedDuplex / WindowsNamedPipe) is unchanged, so reads after a sentclose_notifykeep working.Verification
New test in
test/js/bun/http/proxy.test.ts(test.skipIf(!isASAN), the UAF is only deterministic under ASAN): fails on an unfixed ASAN debug build with the heap-use-after-free above, passes with the fix. Fullproxy.test.ts(46 tests) plusnode-tls-connect,node-tls-upgrade,node-tls-duplex-close-throw-uaf,node-tls-socket-allow-half-open-option,node-tls-server,fetch-tls-cert, andnode-https-checkServerIdentitysuites pass.Note on the asan-lane CI failure (#32144)
The intermittent LeakSanitizer failure on the x64-asan shard (deferred napi finalizers parked on a never-drained cleanup-hook list at
bun testexit) is being fixed in #32146, which carries the sameglobal_exit()drain plus a hooks-only guard that skips pendingnapi_wrapfinalizers on undrained-loop exits. A subset version of that fix was briefly on this branch (e59bc1d) but without the hooks-only guard it madetest/js/third_party/duckdb/duckdb-basic-usage.test.tsSEGV at exit on the asan lane (build 62135), exactly the failure mode #32146's guard prevents, so it was reverted (61f9e70). This PR is scoped to the proxy-tunnel UAF; its asan lane can still intermittently hit the pre-existing #32144 leak until #32146 lands.Related PRs
.zigreference files, which are no longer compiled; this PR fixes the shipping Rust implementation.mark_close_notified()helper fromProxyTunnel::shutdown(silently setting the flag at one call site, withclose_rawexempted). This PR instead closes the gap insideSSLWrapper::shutdown(true)itself, so every fast-shutdown caller (ProxyTunnel::shutdown,ProxyTunnel::close_raw,UpgradedDuplex::close,WebSocketProxyTunnel::shutdown) gets the same "no callbacks after teardown" guarantee without new wrapper API or a shutdown/close_raw asymmetry. The close callback is fired rather than suppressed, so the error teardown path keeps deliveringon_close->close_and_failexactly once (idempotent viaclosed_notified). Test here is a deterministic single-shot repro (the test proxy reassembles TLS records and flushes tickets + response + close_notify in one write) rather than an iteration loop.