Fix use-after-free in JSReadable*Controller end()/close() when onClose re-enters the event loop

src/codegen/generate-jssink.ts

@@ -452,9 +452,33 @@ JSC_DEFINE_HOST_FUNCTION(${controller}__close, (JSC::JSGlobalObject * lexicalGlo
         return JSC::JSValue::encode(JSC::jsUndefined());
     }
 
+    // Null the native pointer before running any JS. detach() fires the
+    // onClose callback, which can re-enter the event loop and free the
+    // sink (e.g. handle_resolve_stream -> destroy_sink) while ptr is still
+    // live on our stack. Do the native close first, then let detach() run
+    // the JS callback once we no longer need ptr.
+    ${name}__controllerDetached(ptr, JSC::JSValue::encode(controller));
+    controller->m_sinkPtr = nullptr;
+
+    ${name}__close(lexicalGlobalObject, ptr);
+
+    // detach() must still fire onClose (it transitions the direct
+    // ReadableStream to closed/errored and calls underlyingSource.cancel())
+    // even if the native close threw, matching the pre-reorder behaviour.
+    // Stash and rethrow around it; the sink's error wins over any onClose
+    // error.
+    if (JSC::Exception* pending = scope.exception()) [[unlikely]] {
+        if (!scope.tryClearException()) {
+            return {};
+        }
+        controller->detach();
+        (void)scope.tryClearException();
+        scope.throwException(lexicalGlobalObject, pending);
+        return {};
+    }
+
     controller->detach();
     RETURN_IF_EXCEPTION(scope, {});
-    ${name}__close(lexicalGlobalObject, ptr);
     return JSC::JSValue::encode(JSC::jsUndefined());
 }
 
@@ -475,9 +499,34 @@ JSC_DEFINE_HOST_FUNCTION(${controller}__end, (JSC::JSGlobalObject * lexicalGloba
         return JSC::JSValue::encode(JSC::jsUndefined());
     }
 
+    // Null the native pointer before running any JS. detach() fires the
+    // onClose callback, which can re-enter the event loop and free the
+    // sink (e.g. handle_resolve_stream -> destroy_sink) while ptr is still
+    // live on our stack. Do the native end first, then let detach() run
+    // the JS callback once we no longer need ptr.
+    ${name}__controllerDetached(ptr, JSC::JSValue::encode(controller));
+    controller->m_sinkPtr = nullptr;
+
+    auto result = ${name}__endWithSink(ptr, lexicalGlobalObject);
+
+    // detach() must still fire onClose (it transitions the direct
+    // ReadableStream to closed/errored and calls underlyingSource.cancel())
+    // even if the native end threw, matching the pre-reorder behaviour.
+    // Stash and rethrow around it; the sink's error wins over any onClose
+    // error.
+    if (JSC::Exception* pending = scope.exception()) [[unlikely]] {
+        if (!scope.tryClearException()) {
+            return {};
+        }
+        controller->detach();
+        (void)scope.tryClearException();
+        scope.throwException(lexicalGlobalObject, pending);
+        return {};
+    }
+
     controller->detach();
     RETURN_IF_EXCEPTION(scope, {});
-    return ${name}__endWithSink(ptr, lexicalGlobalObject);
+    return result;
 }
 
 extern "C" JSC::EncodedJSValue ${name}__getInternalFd(void* sinkPtr);

test/js/bun/http/serve-direct-readable-stream.test.ts

@@ -1,5 +1,6 @@
 import { sleep } from "bun";
 import { expect, test } from "bun:test";
+import { bunEnv, bunExe, isASAN } from "harness";
 
 test("HTTPResponseSink displays correct message", async () => {
   let leakedCtrl: any;
@@ -27,3 +28,89 @@ test("HTTPResponseSink displays correct message", async () => {
   );
   expect(() => leakedCtrl.write.call({}, "c")).toThrow("Expected HTTPResponseSink");
 });
+
+// Sentry BUN-2WJA / BUN-2WKB: JSReadable*Controller.end() ran the onClose
+// callback (via detach()) before calling endWithSink() on the stashed sink
+// pointer. If the stream's pull() promise had already settled, the queued
+// on_resolve_stream reaction frees the sink when microtasks drain during
+// onClose, leaving endWithSink() to dereference a freed HTTPServerWritable.
+//
+// The repro forces the microtask drain from inside the stream's cancel()
+// callback (which is what detach()'s onClose invokes for a direct stream).
+// Under ASAN this is a heap-use-after-free without the fix; in release it
+// segfaults on the scrubbed buffer pointer.
+test.skipIf(!isASAN)(
+  "controller.end() after pull() resolved does not use the sink after free",
+  async () => {
+    const fixture = `
+    const { drainMicrotasks } = require("bun:jsc");
+
+    const big = Buffer.alloc(128 * 1024, 0x61);
+    let capturedController;
+    let resolvePull;
+    const pullSettled = Promise.withResolvers();
+
+    const server = Bun.serve({
+      port: 0,
+      fetch() {
+        return new Response(
+          new ReadableStream({
+            type: "direct",
+            pull(controller) {
+              capturedController = controller;
+              controller.write(big);
+              const p = new Promise(r => { resolvePull = r; });
+              p.then(() => pullSettled.resolve());
+              return p;
+            },
+            cancel() {
+              // Reached from controller.end() -> detach() -> onClose.
+              // Draining here runs on_resolve_stream, which destroys the
+              // native sink while endWithSink() still holds a pointer to it.
+              drainMicrotasks();
+            },
+          }),
+        );
+      },
+    });
+
+    const res = await fetch(server.url);
+    const reader = res.body.getReader();
+    // Read the body to completion so the client never applies backpressure
+    // and the server-side write drains without parking a pending_flush.
+    const drained = (async () => { while (!(await reader.read()).done); })();
+
+    // Wait until pull() has been invoked and the controller is live.
+    while (!resolvePull) await Bun.sleep(0);
+
+    // Queue on_resolve_stream: pull()'s promise -> .then(() => {}) wrapper
+    // inside readDirectStream -> then_with_value(on_resolve_stream, ...).
+    resolvePull();
+    await pullSettled.promise;
+
+    // controller.end(): stashes ptr, detach() fires onClose -> cancel()
+    // -> drainMicrotasks() -> on_resolve_stream frees the sink, then
+    // endWithSink(ptr) runs on the freed allocation.
+    capturedController.end();
+
+    await drained;
+    server.stop(true);
+    console.log("ok");
+  `;
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", fixture],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect({ stdout, stderr, exitCode }).toEqual({
+      stdout: "ok\n",
+      stderr: "",
+      exitCode: 0,
+    });
+  },
+  30_000,
+);