Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/runtime/api/JSTranspiler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -500,9 +500,16 @@ pub const TransformTask = struct {
var ast_scope = ast_memory_allocator.enter(allocator);
defer ast_scope.exit();

// Log message text/locations are allocated from the arena during
// parsing/printing, which is freed when this function returns.
// Use a temporary log and deep-copy into `this.log` so `then()` can
// safely read the messages on the JS thread.
var log = logger.Log.init(allocator);
log.level = this.log.level;
defer bun.handleOom(log.cloneToWithRecycled(&this.log, true));
Comment on lines +507 to +509

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit (pre-existing pattern, not blocking): cloneToWithRecycled() allocates the string-builder backing buffer and notes_buf from bun.default_allocator, but Log.deinit() only frees the msgs ArrayList — so each async transform() that emits parse errors now leaks those two small allocations. This is the same leak that already exists in RuntimeTranspilerStore.zig:326 (and other cloneToWithRecycled callers), and trading the UAF for a bounded error-path leak is the right call here; the proper fix belongs in logger.zig (have Log track/free the cloned buffers) as a follow-up.

Extended reasoning...

What the bug is

cloneToWithRecycled() (src/logger.zig:797-823) deep-copies log messages by computing the total byte length of all message text/locations, calling string_builder.allocate(other.msgs.allocator) for one contiguous string buffer, and other.msgs.allocator.alloc(Data, notes_count) for the notes array. In TransformTask.run(), other is this.log, which was initialized in TransformTask.create() with logger.Log.init(bun.default_allocator) — so both allocations come from bun.default_allocator.

The cloned Msg structs hold slices into these two buffers, but the Log struct itself (logger.zig:604-610) has no field tracking the backing allocations. Log.deinit() (logger.zig:835-836) is just msgs.clearAndFree(), which frees the ArrayList(Msg) storage but never frees the string-builder buffer or the notes buffer. TransformTask.deinit() calls this.log.deinit() and nothing else touches these buffers, so they leak.

Code path

  1. TransformTask.create()this.log = logger.Log.init(bun.default_allocator), so this.log.msgs.allocator == bun.default_allocator.
  2. TransformTask.run() parses with an arena-backed local log; on scope exit, defer log.cloneToWithRecycled(&this.log, true) runs.
  3. Inside cloneToWithRecycled: string_builder.allocate(this.log.msgs.allocator) and this.log.msgs.allocator.alloc(Data, notes_count) allocate from bun.default_allocator. The local string_builder goes out of scope; the only references to its buffer are the Msg.data.text / .location slices.
  4. then() calls this.log.toJS()BuildMessage.createMsg.clone which allocator.dupe()s the strings again into JS-owned memory. Ownership of the original buffers is never transferred.
  5. TransformTask.deinit()this.log.deinit()msgs.clearAndFree(). The string buffer and notes buffer are never freed.

Why existing code doesn't prevent it

Log simply has no ownership model for the auxiliary buffers cloneToWithRecycled creates — there is no field to store them and deinit() doesn't iterate messages. This is a design gap in logger.zig, not something specific to this call site. The identical leak already exists at src/bun.js/RuntimeTranspilerStore.zig:326 (the pattern this PR explicitly follows), and in other cloneToWithRecycled callers like bundle_v2.zig / HTMLBundle.zig.

Impact

Small, error-path-only leak: each async transform() call that produces parse errors/warnings leaks one string buffer (sized to the total error text) plus one []Data for notes, from bun.default_allocator. Before this PR the same strings lived in the arena, which was freed (hence the UAF) — so this PR converts a crash into a bounded leak, which is a strict improvement.

Step-by-step proof

Repro: for (let i = 0; i < N; i++) await t.transform('const x: = y;', 'ts').catch(()=>{});

  • Each iteration emits ≥1 error message → cloneToWithRecycled allocates a string buffer of ~tens of bytes from bun.default_allocator.
  • then() rejects, deinit() runs, msgs.clearAndFree() frees the Msg[] array.
  • No code path holds a pointer to the start of the string-builder allocation, and Log has no field for it, so it cannot be freed.
  • RSS grows linearly in N.

How to fix (follow-up, not for this PR)

Fix in logger.zig rather than at each call site: either have Log store the string_builder buffer / notes_buf handles populated by cloneToWithRecycled and free them in Log.deinit(), or have cloneToWithRecycled allocate per-message owned strings that Msg.deinit can free. Given the same pattern is used in RuntimeTranspilerStore, a single fix there cleans up all call sites.


this.transpiler.setAllocator(allocator);
this.transpiler.setLog(&this.log);
this.log.msgs.allocator = bun.default_allocator;
this.transpiler.setLog(&log);

const jsx = if (this.tsconfig != null)
this.tsconfig.?.mergeJSX(this.transpiler.options.jsx)
Expand Down
25 changes: 25 additions & 0 deletions test/js/bun/transpiler/transpiler-async-log-uaf-fixture.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions test/js/bun/transpiler/transpiler-async-log-uaf.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";
import path from "node:path";

// `Bun.Transpiler.transform()` runs parsing on a worker thread using an arena
// allocator. Log messages (text + locations) were allocated from that arena,
// which was freed before the promise was settled on the JS thread, leading to
// a use-after-free when rendering the error. Run the repro in a subprocess so
// an ASAN abort is observed as a test failure rather than killing the runner.
test("async transform with parse errors does not read freed log messages", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), path.join(import.meta.dir, "transpiler-async-log-uaf-fixture.ts")],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect({
stdout: stdout.trim(),
exitCode,
signalCode: proc.signalCode,
asan: stderr.includes("AddressSanitizer"),
}).toEqual({
stdout: "DONE",
exitCode: 0,
signalCode: null,
asan: false,
});
});
Loading