Fix use-after-free in Bun.Transpiler async transform() error messages#30469
Fix use-after-free in Bun.Transpiler async transform() error messages#30469robobun wants to merge 2 commits into
Conversation
TransformTask.run() uses a MimallocArena for parsing that is destroyed when run() returns on the worker thread. Parse error message text is allocated from that arena. When then() later runs on the JS thread and converts the log to a BuildMessage, it reads freed memory. Collect messages in a local arena-backed log and deep-copy them into the task's persistent log (backed by bun.default_allocator) before the arena is destroyed.
|
Updated 5:21 PM PT - May 10th, 2026
❌ @autofix-ci[bot], your commit d20758f has 1 failures in
🧪 To try this PR locally: bunx bun-pr 30469That installs a local version of the PR into your bun-30469 --bun |
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughTransformTask.run now writes transpiler errors to an arena-allocated local_log and defers deep-copying those messages into this.log before freeing the arena. Two concurrent tests verify returned BuildMessage errors and their position metadata remain usable after the arena is freed. ChangesAsync Transpiler Error Lifetime Safety
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Duplicate of #30263 (already approved). |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // `then()` on the JS thread does not read freed memory. | ||
| var local_log = logger.Log.init(allocator); | ||
| local_log.level = this.log.level; | ||
| defer bun.handleOom(local_log.appendToWithRecycled(&this.log, true)); |
There was a problem hiding this comment.
🟡 Nit: appendToWithRecycled(&this.log, true) deep-copies message text into a StringBuilder buffer + notes_buf allocated from bun.default_allocator (logger.zig:739-740), but Log.deinit() only does msgs.clearAndFree() and never frees those buffers, so each failed async transform() now leaks ~tens of bytes. Trading the UAF for this small error-path leak is clearly the right call and matches the established pattern (RuntimeTranspilerStore.zig, BundleThread.zig, ParseTask.zig, etc.) and the known TODO at logger.zig:767 — just flagging for awareness; a real fix belongs in cloneToWithRecycled/Log.deinit, not here.
Extended reasoning...
What the issue is
local_log.appendToWithRecycled(&this.log, true) calls cloneToWithRecycled (src/logger/logger.zig:723-750), which — when recycled == true — performs a deep copy of every message's text, file path, line_text, and notes into freshly-allocated memory owned by other.msgs.allocator. Since this.log was created with logger.Log.init(bun.default_allocator) (JSTranspiler.zig:479), that allocator is bun.default_allocator. Two allocations are made there:
string_builder.allocate(other.msgs.allocator)at logger.zig:739 — one contiguous buffer holding all copied string dataother.msgs.allocator.alloc(Data, total_notes_count)at logger.zig:740 — the notes backing array
Neither of these is ever freed.
Why nothing frees it
TransformTask.deinit() calls this.log.deinit(), but Log.deinit() (logger.zig:761-765) only does log.msgs.clearAndFree(), which frees the ArrayList(Msg) backing storage — not the per-message text slices or the shared notes_buf. The codebase explicitly acknowledges this at logger.zig:767 with a TODO noting that "deinit does not de-initialize the log; it clears it."
The consumer side doesn't free it either: then() → log.toJS() → BuildMessage.create() → Msg.clone() → allocator.dupe(u8, text) makes another copy without taking ownership of the original. And because cloneToWithRecycled packs all message text into one shared StringBuilder buffer, individual Msg.deinit() calls couldn't correctly free it anyway — it's a single allocation sliced N ways.
Step-by-step proof
- User calls
await transpiler.transform("const x = @@@", "js"). TransformTask.run()createsarenaandlocal_log = logger.Log.init(arena.allocator()).- Parser fails and pushes a
Msgintolocal_logwhosedata.text = "Expected identifier but found \"@\"",location.file = "input.js",location.line_text = "const x = @@@"— all arena-allocated. run()returns. Thedeferfires:local_log.appendToWithRecycled(&this.log, true).cloneToWithRecycledcounts ~55 bytes of string data, callsstring_builder.allocate(bun.default_allocator)→ one ~55-byte heap buffer, plusnotes_buf = bun.default_allocator.alloc(Data, 0)(zero-length, so no real alloc for notes here). The Msg inthis.lognow points into that heap buffer.- Arena is destroyed (fine — nothing points into it anymore).
- JS thread:
then()readsthis.log, builds aBuildMessageby duping the text again, thenthis.deinit()runs. this.log.deinit()→msgs.clearAndFree()frees the 1-elementArrayList(Msg)storage. The ~55-byte StringBuilder buffer from step 5 is never passed tobun.default_allocator.free. Leaked.
The regression test runs 50 failing transforms, so it leaks ~50 × ~55 bytes ≈ 2-3 KB total. On the success path local_log has 0 messages, string_builder.cap == 0, and alloc(u8, 0) doesn't actually allocate, so there's no leak when parsing succeeds.
Before vs. after this PR
Before: error text lived in the arena and was freed by arena.deinit() — no leak, but then() then read freed memory (the UAF this PR fixes). After: error text lives in bun.default_allocator and is never freed — no UAF, small leak. This is unambiguously the right trade.
Why this is a nit, not a blocker
This exact pattern — appendToWithRecycled/cloneToWithRecycled(.., true) into a default_allocator-backed Log that is later only .deinit()'d — is already used identically in 10+ places: RuntimeTranspilerStore.zig:326, BundleThread.zig:153/160, ParseTask.zig:356/367/378, bundle_v2.zig:3639/3787, HTMLBundle.zig:330, npm.zig:1745, Installer.zig:172. The PR is following established codebase convention, and the limitation is documented at logger.zig:767. The leak is error-path only and tiny.
How to fix (out of scope here)
The proper fix is in the logger module: have cloneToWithRecycled stash the StringBuilder buffer pointer + notes_buf on the destination Log (e.g. an owned_buffers list) and have Log.deinit() free them. That would fix this call site and the dozen others simultaneously. Nothing actionable in JSTranspiler.zig itself.
1 participant
What does this PR do?
Fixes a use-after-free in
Bun.Transpiler#transform()(the async variant) when parsing fails.TransformTask.run()runs on a worker thread and uses aMimallocArenaas the transpiler's allocator for parsing. Error message text produced by the parser is allocated from that arena. The arena is destroyed whenrun()returns. Later,then()runs on the JS thread and callslog.toJS()→BuildMessage.create()→Msg.clone()→allocator.dupe(u8, text), which reads the freed arena memory.Under ASAN this shows as
use-after-poisoninData.clone; in release builds the rejection'sBuildMessagecontains garbage bytes instead of the real error message.Fix
Collect parse messages in a local arena-backed log and deep-copy them into the task's persistent log (backed by
bun.default_allocator) viaappendToWithRecycled(..., true)in adeferthat runs before the arena is destroyed.How did you verify your code works?
Minimal repro (crashes under ASAN before, garbage message text in release before):
Added a regression test in
test/js/bun/transpiler/transpiler-async-error-uaf.test.tsthat asserts the rejection is aBuildMessagewith the correctmessageandposition. The test fails on currentmain(receives garbage bytes like"H)yôP)yôX)yô") and passes with this fix. Existing transpiler tests continue to pass.