From be6d3ca2d8204a7c4b696ede9beed267841b2e67 Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Wed, 11 Feb 2026 20:31:12 +0000 Subject: [PATCH] fix(shell): prevent use-after-free in runFromJS error path When `setupIOBeforeRun()` fails in `runFromJS`, `#deinitFromExec()` was called which immediately frees the interpreter via `allocator.destroy(this)`. Since the interpreter is a GC-managed object (`JSShellInterpreter`), the GC finalizer later calls `deinitFromFinalizer()` on already-freed memory, causing a segfault. Replace `#deinitFromExec()` with `#derefRootShellAndIOIfNeeded(true)` which cleans up runtime resources (IO, shell env) and sets `cleanup_state` to `.runtime_cleaned`, letting the GC finalizer safely handle the actual struct destruction. Closes #26907 Co-Authored-By: Claude --- src/shell/interpreter.zig | 2 +- test/regression/issue/26907.test.ts | 80 +++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 test/regression/issue/26907.test.ts diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig index cc79cb8971..18fa8d7845 100644 --- a/src/shell/interpreter.zig +++ b/src/shell/interpreter.zig @@ -1154,7 +1154,7 @@ pub const Interpreter = struct { _ = callframe; // autofix if (this.setupIOBeforeRun().asErr()) |e| { - defer this.#deinitFromExec(); + this.#derefRootShellAndIOIfNeeded(true); const shellerr = bun.shell.ShellErr.newSys(e); return try throwShellErr(&shellerr, .{ .js = globalThis.bunVM().event_loop }); } diff --git a/test/regression/issue/26907.test.ts b/test/regression/issue/26907.test.ts new file mode 100644 index 0000000000..49eb6c9841 --- /dev/null +++ b/test/regression/issue/26907.test.ts @@ -0,0 +1,80 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +// Regression test for #26907: use-after-free in shell interpreter's runFromJS error path. +// When setupIOBeforeRun() fails (e.g., stdout handle is invalid on Windows), the interpreter +// must NOT call allocator.destroy(this) directly, since the GC finalizer will later try to +// access the freed memory. Instead, it should clean up runtime resources via +// derefRootShellAndIOIfNeeded and let the GC handle destruction. +// +// The bug is most reliably triggered on Windows where stdout can be unavailable, +// causing setupIOBeforeRun() -> dup(stdout) to fail. On Linux, dup rarely fails. +test("shell interpreter does not crash on GC after shell error", async () => { + // Run many shell commands and force GC to stress the interpreter lifecycle. + // On Windows with the bug, this would segfault during GC sweep when the + // finalizer accesses already-freed interpreter memory. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + async function main() { + const promises = []; + for (let i = 0; i < 50; i++) { + promises.push(Bun.$\`echo hello\`.quiet().catch(() => {})); + } + await Promise.all(promises); + // Force GC multiple times to trigger finalizers + Bun.gc(true); + Bun.gc(true); + process.stderr.write("OK\\n"); + } + main(); + `, + ], + env: bunEnv, + stderr: "pipe", + stdout: "pipe", + }); + + const [_stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("OK"); + expect(exitCode).toBe(0); +}); + +test("shell interpreter handles closed stdout without crashing", async () => { + // Close stdout before running shell commands to trigger setupIOBeforeRun failure. + // On Windows this reliably triggers the error path; on Linux dup may still succeed. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const fs = require("fs"); + fs.closeSync(1); + async function main() { + try { + await Bun.$\`echo hello\`; + } catch (e) { + // Error is expected when stdout is unavailable + } + // Force GC to trigger finalizer - this would segfault before the fix on Windows + Bun.gc(true); + Bun.gc(true); + process.stderr.write("OK\\n"); + } + main(); + `, + ], + env: bunEnv, + stderr: "pipe", + stdout: "pipe", + }); + + const [_stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("OK"); + // Process must not segfault (exit code 139 on Linux, non-zero on Windows) + expect(exitCode).toBe(0); +});