http: fix use-after-free when proxy tunnel closes after response completes mid-read

src/uws/lib.rs

@@ -580,24 +580,26 @@ pub mod ssl_wrapper {
             };
             // we already sent the ssl shutdown
             if Self::r(this).flags.sent_ssl_shutdown() || Self::r(this).flags.fatal_error() {
-                if fast_shutdown && !Self::r(this).flags.received_ssl_shutdown() {
-                    // The peer went away (raw EOF / destroy) after we had
-                    // already sent our shutdown, and its close_notify will
-                    // never arrive. A fast shutdown means we are done for
-                    // sure: mark received and run the close callback so the
-                    // owner's teardown (UpgradedDuplex::on_close ->
-                    // DuplexUpgradeContext::on_close -> deinit) actually
-                    // happens. Without this, a TLS-over-duplex socket whose
-                    // peer half-closes at the TCP level never tears down and
-                    // leaks its whole context graph (LeakSanitizer caught
-                    // this in test-tls-js-stream / test-tls-inception).
+                if fast_shutdown {
+                    // A fast shutdown is a full teardown — the owner calls it
+                    // right before detaching/freeing handlers.ctx (proxy tunnel
+                    // on response-complete; TLS-over-duplex on raw EOF).
+                    //
+                    // Run the close callback now, regardless of whether the
+                    // peer's close_notify has arrived: if it HAS (processed
+                    // mid handle_reading, which sets sent_ssl_shutdown before
+                    // flushing the final decrypted bytes), closed_notified may
+                    // still be unset and handle_reading's deferred
+                    // trigger_close_callback would otherwise fire on_close
+                    // into the freed ctx; if it has NOT (peer went away after
+                    // our shutdown), the TLS-over-duplex teardown chain
+                    // (UpgradedDuplex::on_close -> DuplexUpgradeContext::on_close
+                    // -> deinit) never runs and leaks the whole context graph.
                     // trigger_close_callback is idempotent (closed_notified).
                     Self::r(this).flags.set_received_ssl_shutdown(true);
                     Self::r(this).trigger_close_callback();
                     // Do not read self after the close callback: the owner's
-                    // teardown chain has started (deinit is deferred to the
-                    // next tick today, but nothing here should rely on that).
-                    // The answer is known - we just set it.
+                    // teardown chain has started.
                     return true;
                 }
                 return Self::r(this).flags.received_ssl_shutdown();

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";
@@ -916,6 +916,126 @@ 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]);
+    if (exitCode !== 0) console.error("stderr:", stderr);
+    expect(stdout).toBe("hello\n");
+    expect(exitCode).toBe(0);
+  },
+);
+
 // Sentry BUN-2V7Z: debug_assert!(!socket.is_shutdown()/is_closed()) in the
 // ProxyHeaders arm of on_writable (and the matching assert in
 // send_initial_request_payload) fired when the outer proxy socket died

test/regression/issue/30205.test.ts

@@ -99,6 +99,11 @@ describe.skipIf(!canBuildNodeAddons())("#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.skipIf(!canBuildNodeAddons())("#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.");