transpiler: fix use-after-free in async transform() error path#30744
Closed
robobun wants to merge 1 commit into
Closed
transpiler: fix use-after-free in async transform() error path#30744robobun wants to merge 1 commit into
robobun wants to merge 1 commit into
Conversation
TransformTask.run() parses with an arena allocator on a worker thread and frees the arena on return. Parser/lexer error messages allocate their text and notes in that arena; the Msg structs were shallow-copied into this.log, so when then() ran on the JS thread and called log.toJS() -> Msg.clone() it read freed arena memory (ASAN use-after-poison). Deep-copy the log into bun.default_allocator in a defer that runs before arena.deinit() so the rejection path sees stable strings.
Collaborator
Author
|
Updated 3:50 PM PT - May 14th, 2026
❌ @robobun, your commit 627a54e has 15 failures in
🧪 To try this PR locally: bunx bun-pr 30744That installs a local version of the PR into your bun-30744 --bun |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR fixes a use-after-free bug in the Bun transpiler's async error handling. When ChangesTranspiler UAF Prevention
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
+498
to
+507
| // Log message text/notes may be allocated in the arena by the parser. | ||
| // Deep-copy them into default_allocator before the arena is freed so | ||
| // `then()` can safely read them on the JS thread. | ||
| defer if (this.log.msgs.items.len > 0) { | ||
| var cloned = logger.Log.init(bun.default_allocator); | ||
| cloned.level = this.log.level; | ||
| this.log.cloneToWithRecycled(&cloned, true) catch {}; | ||
| this.log.msgs.clearAndFree(); | ||
| this.log = cloned; | ||
| }; |
Contributor
There was a problem hiding this comment.
🔴 This fix is applied only to src/runtime/api/JSTranspiler.zig, which per CLAUDE.md is a non-compiled porting reference — the shipped implementation is src/runtime/api/JSTranspiler.rs. TransformTask::run() in the .rs file (lines 771-863) has the identical arena-then-drop pattern (it even launders the arena to 'static via unsafe { bun_ptr::detach_lifetime_ref(&arena) } at :787) and is untouched by this PR, so the UAF described in the title is not actually fixed in the binary. The clone-out-of-arena logic needs to be ported into TransformTask::run() in JSTranspiler.rs before arena drops.
Extended reasoning...
What's wrong
CLAUDE.md:141 is explicit: ".zig files alongside many .rs files … are the original Zig implementation, kept only as a porting reference — they are not compiled and not shipped. New code goes in .rs. … Never add new behavior to a .zig file." src/CLAUDE.md repeats this, there is no top-level build.zig, and the build is driven by Cargo (scripts/build/rust.ts produces libbun_rust.a). The active implementation is src/runtime/api/JSTranspiler.rs (line 1814: // ported from: src/runtime/api/JSTranspiler.zig), and src/runtime/dispatch.rs imports crate::api::JSTranspiler::AsyncTransformTask from the Rust module — that is what runs at runtime.
This PR's only source change is the new defer block in JSTranspiler.zig. Since that file is not compiled into the binary, the change is a no-op for shipped behavior, and the PR title "fix use-after-free in async transform() error path" is not satisfied.
The Rust path has the same shape
TransformTask::run() in JSTranspiler.rs:771-863 mirrors the Zig structure exactly:
- :776 —
let arena = Arena::new();(drops at end ofrun()) - :784-787 —
// SAFETY: arena outlives every use through self.transpiler in this fn bodyfollowed byself.transpiler.set_arena(unsafe { bun_ptr::detach_lifetime_ref(&arena) }), laundering the borrow to'static - :824 —
self.transpiler.parse(parse_options, None)runs the parser/lexer with that arena - no clone of
self.logbefore the function returns andarenadrops - :887 in
then()—self.log.to_js(self.global, "Transform failed")reads the log on the JS thread
The SAFETY comment's invariant ("outlives every use … in this fn body") is precisely the assumption that breaks if any Msg payload escapes via self.log into then().
Why existing Rust code doesn't obviously prevent it
bun_ast::Location stores file and line_text as Cow<'static, [u8]>, and the PORT NOTE at src/ast/lib.rs:780-783 explicitly states the "Borrowed arm may carry a lifetime-erased view into Source.contents (see init_or_null, css_parser.rs, error.rs, JSBundler.rs)". Location::init (:883-893) and init_or_null (:897-937) both construct Cow::Borrowed views. The hand-written Clone impl deep-dupes specifically because the derived clone "would re-borrow that pointer". So the Rust Log is designed under the same hazard model as Zig: borrowed slices that must be deep-copied before their backing storage goes away.
One nuance: some Rust call sites already own their bytes (e.g. init_or_null at :934 does Cow::Owned(...to_vec()) for line_text, and Data.text from alloc_print is heap-owned), so the Rust path may dangle on fewer fields than Zig did. But Location.file at :900/:917 is still Cow::Borrowed(source.path.text), and Location::init accepts borrowed line_text. Given the explicit detach_lifetime_ref lifetime erasure and the PORT NOTE acknowledging borrowed arena views, the Rust side cannot be assumed safe without the same clone-before-drop step.
Step-by-step proof that the fix doesn't reach the binary
- User calls
new Bun.Transpiler().transform("const @@@ …"). - Dispatch goes through
src/runtime/dispatch.rs→crate::api::JSTranspiler(the Rust module).JSTranspiler.zigis not part of the crate graph. TransformTask::create(Rust) builds the task; worker thread callsTransformTask::run()atJSTranspiler.rs:771.run()createsarena, launders it to'static, parses; lexer pushes errorMsgs intoself.logwhoseLocationfields mayCow::Borrowedarena-/source-backed bytes (perlib.rs:780-783).run()returns →arenadrops. There is no clone step — the new Zigdeferis in a file that was never compiled.- JS thread runs
then()→self.log.to_js(...)at :887 →BuildMessage::create→Msg::clone→Location::clonereads the (potentially freed/lifetime-erased) bytes.
The new test may pass anyway because Rust's bump arena does not poison freed pages the way Zig's debug MimallocArena does, so a green CI run here is not evidence the UAF is gone.
Impact
The PR is effectively a doc/test-only change against the shipped binary. Async transform() rejections in release builds remain exposed to the same dangling-read described in the PR body, and a future arena-poisoning/ASAN run on the Rust path would still trip.
Fix
Port the same logic into TransformTask::run() in src/runtime/api/JSTranspiler.rs — before arena drops (i.e. before returning), deep-clone self.log into the global allocator (the Rust analogue of cloneToWithRecycled, e.g. iterate self.log.msgs and Msg::clone() each into a fresh Log, then swap). Optionally also revert/drop the .zig edit since per CLAUDE.md new behavior should not be added there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Bun.Transpiler#transform()(async) could read freed arena memory when the parse produced errors, tripping an ASAN use-after-poison in debug builds.Why
TransformTask.run()runs the parser on a worker thread using a localMimallocArenaas the transpiler allocator, then frees the arena on return. The parser/lexer allocate error-message text and notes with that arena allocator. ThoseMsgs are shallow-copied intoTransformTask.log(only the outermsgslist usesbun.default_allocator).When
then()later runs on the JS thread and callsthis.log.toJS()→BuildMessage.create→Msg.clone→Data.clone, itdupes the arena-backed text after the arena is already freed.How
Add a
deferinTransformTask.run()(ordered beforearena.deinit()) that deep-copies any log messages intobun.default_allocatorviacloneToWithRecycled(&cloned, true), sothen()reads stable memory.Repro
Found by Fuzzilli.