Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/http/ProxyTunnel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/uws/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
115 changes: 115 additions & 0 deletions test/js/web/fetch/fetch-proxy-tunnel-close-uaf-fixture.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

76 changes: 76 additions & 0 deletions test/js/web/fetch/fetch-proxy-tunnel-close-uaf.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, string | undefined> = { ...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);
});