Bun.password: fix UAF when worker.terminate() lands mid argon2/bcrypt job by robobun · Pull Request #35156 · oven-sh/bun

robobun · 2026-07-22T16:14:54Z

Repro

const { Worker } = require("node:worker_threads");
const src = `const { parentPort } = require("node:worker_threads");
const lane = f => (async () => { for (;;) { try { await f(); } catch {} } })();
for (let i = 0; i < 3; i++) lane(() => Bun.password.hash("hunter2", { algorithm: "argon2id", memoryCost: 1 << 15, timeCost: 3 }));
for (let i = 0; i < 3; i++) lane(() => Bun.password.hash("pw", { algorithm: "bcrypt", cost: 10 }));
parentPort.postMessage("up");`;
for (let r = 0; r < 12; r++) {
  const w = new Worker(src, { eval: true });
  await new Promise(res => w.once("message", res));
  await Bun.sleep(60 + (r * 41) % 220);
  await w.terminate();
}

Stock release: panic / SIGSEGV on round 0 or 1. Debug+ASAN:

heap-use-after-free  READ of size 8  thread T17 (Bun Pool 5)
  #0 EventLoop::vm_ref                    src/jsc/event_loop.rs
  #1 EventLoop::enqueue_task_concurrent   src/jsc/event_loop.rs
  #2 PasswordJob::<HashOp>::run_owned     src/runtime/crypto/PasswordObject.rs
freed by thread (Worker): WebWorker::shutdown -> dealloc(VirtualMachine)

Cause

PasswordJob::run_owned runs argon2/bcrypt on the shared work pool and posts the completion back through a raw *mut EventLoop captured at schedule time. argon2/bcrypt jobs take hundreds of ms, so worker.terminate() almost always lands mid-compute: the worker's VirtualMachine box (with its embedded EventLoop) is freed, then the pool thread dereferences the freed pointer in enqueue_task_concurrent. Bun.password.verify() shares the same site. Worker process.exit() and an uncaught throw are equal triggers.

Fix

Add a per-VM ShutdownGate (futex-backed counted guest gate, Arc'd so guests may outlive the VM box). PasswordJob clones the Arc on the JS thread and brackets the event_loop dereference on the pool thread with enter()/leave(); WebWorker::shutdown calls close_and_wait() on the gate before the VM dealloc.

Because the gate is entered only for the enqueue itself (not held across the whole hash), terminate() latency is bounded by the microsecond enqueue critical section rather than the argon2 runtime. A pool thread that has not reached the enqueue yet observes the closed gate, drops its completion, and returns; the promise's StrongRef slot (which points into the dead JSC VM's HandleSet) is leaked rather than Drop-dereferenced.

A completion that makes it into the queue under the gate (the microsecond race where a pool thread is mid-enqueue when close_and_wait() is called) would previously be re-queued by the shutdown drain and leak once the worker VM box is dealloc'd. AnyTask gains an optional dispose slot that release_queued_tasks_for_shutdown calls (JS thread, VM still alive, so JSPromiseStrong::Drop is safe); PasswordResult's dispose is a plain box drop.

The gate primitive, its VM wiring, and the AnyTask dispose slot are the minimal subset of the design in #34154; this PR applies it to the Bun.password site only. The other cross-thread producers (fetch, node:fs async, zlib, S3, napi async work, etc.) remain as on main and are covered by #34154 / #32071.

Verification

New test in test/js/web/workers/worker-terminate-lifetime.test.ts:

USE_SYSTEM_BUN=1: child crashes on round 0 (release panic)
bun bd without src/ fix: child aborts with the ASAN heap-use-after-free above
bun bd with fix: passes (4 rounds under ASAN, ~9s)
bun bd test test/js/bun/util/password.test.ts: 68 pass / 8 skip
cargo test -p bun_threading shutdown_gate: 2 pass
bun run rust:check-all: 10/10 targets ok

The pre-existing dns.lookup test in the same file fails on current main (4104-byte node_fs_binding::Binding leak) with or without this change.

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/workers/worker-terminate-lifetime.test.ts

…nate PasswordJob::run_owned runs argon2/bcrypt on the shared work pool and enqueues the completion back through a raw *mut EventLoop captured at schedule time. worker.terminate() frees the VM box mid-hash, then the pool thread dereferences the freed event loop (heap-use-after-free in EventLoop::enqueue_task_concurrent, stock SIGSEGV on release). Add a per-VM ShutdownGate (futex-backed counted gate, Arc'd so guests may outlive the VM box). PasswordJob clones the Arc on the JS thread and brackets the event_loop dereference with enter()/leave() on the pool thread; worker shutdown close_and_wait()s the gate before the VM dealloc. Because the gate is entered only for the enqueue itself (not held across the whole hash), terminate latency is bounded by the microsecond enqueue critical section rather than the argon2 runtime. A pool thread that loses the race drops its completion and leaks the promise's HandleSlot (points into the dead JSC VM; Drop would UAF).

coderabbitai · 2026-07-22T16:15:01Z

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 709d9635-1f3a-4ce6-abeb-045917b49756

📥 Commits

Reviewing files that changed from the base of the PR and between 47597ab and a23e86f.

📒 Files selected for processing (16)

src/bundler/bundle_v2.rs
src/event_loop/AnyTask.rs
src/jsc/AsyncModule.rs
src/jsc/VirtualMachine.rs
src/jsc/any_task_job.rs
src/jsc/event_loop.rs
src/jsc/web_worker.rs
src/runtime/crypto/PasswordObject.rs
src/runtime/dns_jsc/dns.rs
src/runtime/node/node_fs_stat_watcher.rs
src/runtime/socket/WindowsNamedPipeContext.rs
src/runtime/socket/socket_body.rs
src/runtime/valkey_jsc/js_valkey.rs
src/threading/ShutdownGate.rs
src/threading/lib.rs
test/js/web/workers/worker-terminate-lifetime.test.ts

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-07-22T16:20:29Z

Found 4 issues this PR may fix:

Worker create+terminate cycle aborts process after ~100k–900k iterations on macOS arm64 #30421 - Worker create+terminate cycle aborts process after ~100k-900k iterations; the pool-thread-races-worker.terminate() lifetime hazard that ShutdownGate addresses could be a contributing cause
panic: Segmentation fault at address 0xD — "multiple threads are crashing" under Worker spawn/terminate churn (1.3.14, long-running server) #31880 - Segfault under Worker spawn/terminate churn in long-running server; crash signatures match the freed-EventLoop/VM UAF pattern this PR fixes
Worker lifetime: carry a generation token with cross-thread VM handles (follow-up to #32071) #32073 - Worker lifetime: carry a generation token with cross-thread VM handles; explicitly lists PasswordObject as needing conversion, which this PR ShutdownGate accomplishes
Worker & worker_threads stability tracking issue #15964 - Worker and worker_threads stability tracking issue; calls out do not attempt to reference an event loop of a closed Worker as an open TODO, which is precisely the class of bug the ShutdownGate addresses

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #30421
Fixes #31880
Fixes #32073
Fixes #15964

🤖 Generated with Claude Code

robobun · 2026-07-22T16:22:40Z

Related but not auto-closable by this PR:

Worker create+terminate cycle aborts process after ~100k–900k iterations on macOS arm64 #30421, panic: Segmentation fault at address 0xD — "multiple threads are crashing" under Worker spawn/terminate churn (1.3.14, long-running server) #31880: general worker terminate churn crashes. This PR fences only the Bun.password work-pool completion; the other cross-thread producers (fetch, node:fs async, zlib, S3, napi async work) still enqueue through raw pointers and remain covered by Fix use-after-free when worker.terminate() races in-flight fetch/work-pool completions #34154 / Don't enqueue to a terminated worker's freed event loop from other threads #32071. These issues likely reduce in frequency for workloads that use Bun.password in workers but are not fully fixed.
Worker lifetime: carry a generation token with cross-thread VM handles (follow-up to #32071) #32073: design-tracking issue for generation-token handles across all producers; this PR uses a gate rather than a generation token and covers one site.
Worker & worker_threads stability tracking issue #15964: umbrella worker stability tracker.

Leaving these open.

robobun · 2026-07-22T16:22:44Z

^{Updated 1:30 PM PT - Jul 22nd, 2026}

❌ @robobun, your commit a23e86f has 3 failures in Build #77992 (All Failures):

step failed outside runner - exit 1 on 🍎 26 aarch64 - test-bun
test/js/node/test/parallel/test-gc-http-client-connaborted.js - timeout on 🐧 25.04 x64
test/js/node/test/parallel/test-gc-http-client-connaborted.js - timeout on 🐧 13 x64
test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 13 x64
test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 25.04 x64

🧪 To try this PR locally:

bunx bun-pr 35156

That installs a local version of the PR into your bun-35156 executable, so you can run:

bun-35156 --bun

github-actions · 2026-07-22T16:24:17Z

This PR may be a duplicate of:

Fix use-after-free when worker.terminate() races in-flight fetch/work-pool completions #34154 - Superset that introduces the same ShutdownGate primitive and applies it to PasswordObject.rs plus all other cross-thread producers; Bun.password: fix UAF when worker.terminate() lands mid argon2/bcrypt job #35156 is explicitly described as the "minimal subset" of this PR
Don't enqueue to a terminated worker's freed event loop from other threads #32071 - Comprehensive registry-based fix (VmHandle/LoopHandle) covering all cross-thread producers including PasswordObject.rs, subsuming both Bun.password: fix UAF when worker.terminate() lands mid argon2/bcrypt job #35156 and Fix use-after-free when worker.terminate() races in-flight fetch/work-pool completions #34154
worker: fence async crypto (AnyTaskJob) completions against terminate() freeing the VM #35154 - Fixes the same UAF-on-terminate-during-crypto-jobs class of bug via AnyTaskJob fencing, using a similar gate mechanism for the adjacent node:crypto entrypoint

🤖 Generated with Claude Code

robobun · 2026-07-22T16:25:02Z

On the duplicate-PR suggestions:

Fix use-after-free when worker.terminate() races in-flight fetch/work-pool completions #34154 and Don't enqueue to a terminated worker's freed event loop from other threads #32071 are already called out in the PR body; both are supersets that cover every cross-thread producer. This PR is the Bun.password slice, using the ShutdownGate primitive from Fix use-after-free when worker.terminate() races in-flight fetch/work-pool completions #34154 so that PR can rebase on top.
worker: fence async crypto (AnyTaskJob) completions against terminate() freeing the VM #35154 is not a duplicate: it fixes the adjacent AnyTaskJob site (crypto.scrypt/pbkdf2/hkdf, Bun.secrets, Bun.zstd*), which does not go through PasswordJob. The two PRs are complementary but will merge-conflict on the VM gate field and the web_worker.rs close call, since worker: fence async crypto (AnyTaskJob) completions against terminate() freeing the VM #35154 introduces a separate Arc<RwLock<bool>> gate. Whichever lands first, the other should rebase onto its gate; ShutdownGate here is the same type Fix use-after-free when worker.terminate() races in-flight fetch/work-pool completions #34154 adds.

claude

No bugs found; the earlier test-wiring nit is addressed in 61bab23. Deferring to a human because this introduces a new cross-thread synchronization primitive (ShutdownGate) wired into VirtualMachine init/destroy and worker teardown ordering — memory-safety-critical paths that warrant maintainer eyes, and the primitive/placement is a design commitment that #34154 / #35154 will rebase onto.

What was reviewed:

ShutdownGate enter/leave/close ordering and the optimistic-add-then-undo race against close_and_wait's re-read loop — looks sound; futex wake covers all closers.
run_owned gate-closed path: JSPromiseStrong is ManuallyDrop'd (not dropped) so the dead HandleSet slot isn't dereferenced; KeepAlive has no Drop; the Box<Self> drop still zero-wipes password/prev_hash.
destroy() backstop: main-VM uses close_without_waiting (box never freed), worker path re-closes-and-waits idempotently; the Option::take() drops the VM's Arc ref explicitly since worker boxes bypass Drop.
Gate placement in WebWorker::shutdown sits after markShuttingDown and before release_queued_tasks_for_shutdown, so a gated enqueue that lands is still reclaimed.

Extended reasoning...

Overview

The PR fixes a heap-use-after-free where PasswordJob::run_owned (running argon2/bcrypt on the shared work pool) posts its completion through a raw *mut EventLoop after worker.terminate() has freed the worker's VirtualMachine box. It introduces a new bun_threading::ShutdownGate primitive (futex-backed counted guest gate in an Arc), stores one on every VirtualMachine, closes-and-waits on it in WebWorker::shutdown before the VM dealloc, and has PasswordJob bracket its event_loop dereference with enter()/leave(). Six files touched: the new primitive + export, VM field/init/destroy wiring, the web_worker.rs close call, the PasswordObject.rs producer, and a new spawned-subprocess regression test.

Security risks

None introduced. The change is defensive (prevents UAF). No new user-facing surface, no parsing of untrusted input, no auth/crypto-algorithm changes — the argon2/bcrypt compute path itself is unchanged.

Level of scrutiny

High. This is exactly the category REVIEW.md's memory-safety section is about: cross-thread lifetime, a new hand-rolled synchronization primitive with atomics + futex, and an ordering-sensitive insertion into the worker VM teardown sequence. A subtle bug here would be a UAF or a terminate() hang. The ShutdownGate primitive itself is small and unit-tested, and the enter/leave/close_and_wait interaction looks correct (optimistic fetch_add with undo-on-closed, the closer's while state != CLOSED loop tolerating the transient count, wake(u32::MAX) covering multiple closers). The gate-closed path in run_owned correctly leaks the JSPromiseStrong slot via ManuallyDrop rather than letting Drop deref the dead JSC HandleSet, and KeepAlive having no Drop is stated and matches the codebase. But this is the kind of change where a maintainer who knows the full worker-teardown ordering and the competing #32071 / #34154 / #35154 designs should sign off.

Other factors

The PR is explicitly a slice of #34154's design and will merge-conflict with #35154 (which adds a different Arc<RwLock<bool>> gate for the AnyTaskJob site). A human should decide sequencing and confirm ShutdownGate is the primitive both should converge on.
My earlier nit (wire error/exit to reject the readiness promise) was addressed in 61bab23.
Verification in the PR body is thorough (USE_SYSTEM_BUN fails, unfixed debug+ASAN reproduces the UAF, fixed build passes, existing password tests pass, cargo unit tests pass, rust:check-all clean across 10 targets). CI build #77974 is in progress.
The close_and_wait() in shutdown() runs on the worker thread while the pool thread holds the gate only for the microsecond enqueue — so terminate() latency is not extended by the argon2 runtime, as the PR body claims. I did not find a path where a guest could hold the gate across something that blocks on the worker thread (which would deadlock).

…ed PasswordResults A completion that makes it into the queue under the gate is re-queued (AnyTask was not consumable at shutdown) and leaks once the worker VM box is dealloc'd. Give AnyTask an optional dispose fn and have release_queued_tasks_for_shutdown call it (JS thread, VM alive, so JSPromiseStrong::Drop is safe there); PasswordResult's dispose is a plain box drop.

robobun · 2026-07-22T17:18:43Z

CI on a23e86f: the worker-terminate-lifetime.test.ts ASAN-lane leak from the previous build is resolved by the AnyTask dispose slot. Remaining failures are unrelated to this diff:

test/js/node/test/parallel/test-net-connect-memleak.js and test/js/node/test/parallel/test-gc-http-client-connaborted.js are both also red on main build 77601 (same base commit 47597ab); FinalizationRegistry GC-timing failures, nothing here touches net/http or GC.
:darwin: 26 aarch64 test-bun step failed outside the runner with "number of VMs exceeds the system limit" (CI infra).
bun-install-registry.test.ts (win11 aarch64), serve-protocols.test.ts (debian x64 HTTP3), test-fs-promises-file-handle-readFile.js (debian aarch64), no-orphans.test.ts (darwin x64): all passed on retry.

Diff is ready for review.

robobun · 2026-07-25T18:01:23Z

Generalised into #35767 using the enqueue-by-identifier route (same serialization point as markTerminating(), no wait on in-flight argon2/bcrypt).

alii · 2026-08-12T18:02:30Z

superseded by #37075, password jobs now go through the per-vm handle that teardown closes. repro no longer crashes on main

robobun · 2026-08-12T18:12:11Z

Thanks, confirmed #37075 is merged (9d519e8) and routes PasswordJob through the per-VM handle that teardown closes. Closing this as superseded.

robobun assigned Jarred-Sumner Jul 22, 2026

github-actions Bot added the claude label Jul 22, 2026

clippy: ManuallyDrop instead of mem::forget for leaked promise handle

092a1a7

claude Bot reviewed Jul 22, 2026

View reviewed changes

Comment thread test/js/web/workers/worker-terminate-lifetime.test.ts Outdated

test: wire worker error/exit to reject the readiness promise

61bab23

claude Bot reviewed Jul 22, 2026

View reviewed changes

This was referenced Jul 25, 2026

webcrypto: fence ConcurrentCppTask's post-body unref against worker.terminate() freeing the VM #35758

Closed

worker: post every cross-thread completion by ScriptExecutionContext id so terminate() cannot UAF the freed VM #35767

Closed

github-actions Bot mentioned this pull request Aug 3, 2026

napi: block worker shutdown on in-flight napi_async_work (UAF) #36855

Closed

alii closed this Aug 12, 2026

Navigation Menu

Uh oh!

Bun.password: fix UAF when worker.terminate() lands mid argon2/bcrypt job - #35156

Bun.password: fix UAF when worker.terminate() lands mid argon2/bcrypt job#35156
robobun wants to merge 4 commits into
mainfrom
farm/8656fc50/password-worker-terminate-uaf

robobun commented Jul 22, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading

Review limit reached

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

robobun commented Jul 22, 2026

Uh oh!

robobun commented Jul 22, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

robobun commented Jul 22, 2026

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Jul 22, 2026 •

edited

Loading

Uh oh!

robobun commented Jul 25, 2026

Uh oh!

alii commented Aug 12, 2026

Uh oh!

robobun commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

robobun commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Repro

Cause

Fix

Verification

Uh oh!

coderabbitai Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

robobun commented Jul 22, 2026

Uh oh!

robobun commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

robobun commented Jul 22, 2026

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robobun commented Jul 25, 2026

Uh oh!

alii commented Aug 12, 2026

Uh oh!

robobun commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robobun commented Jul 22, 2026 •

edited

Loading

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading

robobun commented Jul 22, 2026 •

edited

Loading

robobun commented Jul 22, 2026 •

edited

Loading