Skip to content
Merged
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
22 changes: 12 additions & 10 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2510,36 +2510,38 @@ pub fn process_fetch_log(
}

_ => {
// Caps at 256. We heap-allocate the
// exact `len` since `JSValue` is a thin u64 and 256 * 8 B = 2 KiB
// is fine either way, and `Vec` avoids the uninit-array dance.
let len = log.msgs.len().min(256);
let mut errors: alloc::vec::Vec<JSValue> = alloc::vec::Vec::with_capacity(len);
for msg in log.msgs.drain(..len) {
let v = match msg.metadata {
// On-stack array: the conservative GC stack scan is the only
// thing keeping these wrappers alive until
// `create_aggregate_error` stores them. A heap `Vec` is invisible
// to that scan, so a GC triggered by a later `create` could sweep
// the earlier cells and free their native
// `BuildMessage`/`ResolveMessage` out from under us.
let mut errors_stack: [JSValue; 256] = [JSValue::default(); 256];
let len = log.msgs.len().min(errors_stack.len());
for (i, msg) in log.msgs.drain(..len).enumerate() {
errors_stack[i] = match msg.metadata {
bun_ast::Metadata::Build => take(BuildMessage::create(global_this, msg)),
bun_ast::Metadata::Resolve(_) => take(ResolveMessage::create(
global_this,
&msg,
referrer_utf8.slice(),
)),
};
errors.push(v);
}

// C++ `Zig::toString` does `createWithoutCopying`, so the buffer
// must outlive the AggregateError. Mark it global so JSC adopts it
// as an ExternalStringImpl and frees it via `free_global_string`.
let message_text: &'static mut [u8] = bun_core::heap::release(
format!("{} errors building \"{specifier}\"", errors.len())
format!("{len} errors building \"{specifier}\"")
.into_bytes()
.into_boxed_slice(),
);
let mut message = crate::ZigString::init(message_text);
message.mark_global();
*ret = ErrorableResolvedSource::err(
err,
take(global_this.create_aggregate_error(&errors, &message)),
take(global_this.create_aggregate_error(&errors_stack[..len], &message)),
);
}
}
Expand Down
12 changes: 8 additions & 4 deletions src/jsc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1947,12 +1947,16 @@ impl LogJsc for bun_ast::Log {
0 => Ok(JSValue::UNDEFINED),
1 => msg_to_js(&msgs[0], global),
_ => {
let mut errors_stack: Vec<JSValue> = Vec::with_capacity(count);
for msg in &msgs[0..count] {
errors_stack.push(msg_to_js(msg, global)?);
// On-stack array: conservative GC stack scan keeps these
// JSValues alive until `create_aggregate_error` stores them;
// a heap `Vec` is invisible to the scan, so a GC triggered by
// a later `msg_to_js` could sweep the earlier wrappers.
let mut errors_stack: [JSValue; 256] = [JSValue::default(); 256];
for (i, msg) in msgs[0..count].iter().enumerate() {
errors_stack[i] = msg_to_js(msg, global)?;
}
let out = bun_core::ZigString::init(message.as_bytes());
global.create_aggregate_error(&errors_stack, &out)
global.create_aggregate_error(&errors_stack[..count], &out)
}
}
}
Expand Down
37 changes: 18 additions & 19 deletions src/runtime/bake/production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -942,25 +942,24 @@ pub(super) fn build_with_vm(
));
}

let mut css_chunk_js_strings: Vec<JSValue> = vec![JSValue::ZERO; css_chunks_count];
debug_assert_eq!(
pt.bundled_outputs[css_chunks_first..][..css_chunks_count].len(),
css_chunk_js_strings.len()
);
for (output_file, str) in pt.bundled_outputs[css_chunks_first..][..css_chunks_count]
.iter()
.zip(css_chunk_js_strings.iter_mut())
{
// These strings live only in this heap Vec until the route arrays below
// store them; the conservative GC stack scan cannot see a heap buffer, so
// keep each one protected (module evaluation below can trigger GC).
let mut css_chunk_js_strings: Vec<jsc::ProtectedJSValue> = Vec::with_capacity(css_chunks_count);
for output_file in pt.bundled_outputs[css_chunks_first..][..css_chunks_count].iter() {
debug_assert!(output_file.dest_path[0] != b'.');
// CSS chunks must be in contiguous order!!
debug_assert!(output_file.loader.is_css());
*str = BunString::create_format(format_args!(
"{}{}",
BStr::new(public_path),
BStr::new(&output_file.dest_path),
))
.to_js(global)
.map_err(js_err)?;
css_chunk_js_strings.push(
BunString::create_format(format_args!(
"{}{}",
BStr::new(public_path),
BStr::new(&output_file.dest_path),
))
.to_js(global)
.map_err(js_err)?
.protected(),
);
}

// Route URL patterns with parameter placeholders.
Expand Down Expand Up @@ -1075,7 +1074,7 @@ pub(super) fn build_with_vm(
.put_index(
global,
css_file_count,
css_chunk_js_strings[r#ref.get() as usize - css_chunks_first],
css_chunk_js_strings[r#ref.get() as usize - css_chunks_first].value(),
)
.map_err(js_err)?;
css_file_count += 1;
Expand All @@ -1093,7 +1092,7 @@ pub(super) fn build_with_vm(
.put_index(
global,
css_file_count,
css_chunk_js_strings[r#ref.get() as usize - css_chunks_first],
css_chunk_js_strings[r#ref.get() as usize - css_chunks_first].value(),
)
.map_err(js_err)?;
css_file_count += 1;
Expand All @@ -1116,7 +1115,7 @@ pub(super) fn build_with_vm(
.put_index(
global,
css_file_count,
css_chunk_js_strings[r#ref.get() as usize - css_chunks_first],
css_chunk_js_strings[r#ref.get() as usize - css_chunks_first].value(),
)
.map_err(js_err)?;
css_file_count += 1;
Expand Down
51 changes: 50 additions & 1 deletion test/js/bun/resolve/build-error.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { tempDir } from "harness";
import { bunEnv, bunExe, tempDir } from "harness";
import { join } from "node:path";

test("BuildError is modifiable", async () => {
Expand All @@ -19,6 +19,55 @@ test("BuildError is modifiable", async () => {
expect(error!.message).not.toBe(message);
});

test("import with many build errors keeps AggregateError entries alive across GC", async () => {
// process_fetch_log accumulated the BuildMessage wrappers in a heap Vec
// while creating the next ones; the conservative GC stack scan never saw
// them, so a collection triggered mid-loop swept the earlier cells and
// freed their native BuildMessage (use-after-free found by fuzzing).
// 257 duplicate declarations produce 256 log messages, maximizing the
// number of allocations between the first wrapper and the AggregateError.
const dupes = Array.from({ length: 257 }, (_, i) => `const dup = ${i};`).join("\n");
using dir = tempDir("build-error-gc", {
"bad.js": dupes,
"main.js": `
const jobs = [];
for (let i = 0; i < 16; i++) {
jobs.push(
import("./bad.js?v=" + i).then(
() => {
throw new Error("expected rejection");
},
e => {
let n = 0;
for (const err of e.errors ?? []) {
if (typeof err.message === "string") n++;
}
return n;
},
),
);
}
const counts = await Promise.all(jobs);
console.log(JSON.stringify(counts));
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "main.js"],
env: { ...bunEnv, BUN_JSC_slowPathAllocsBetweenGCs: "100" },
cwd: String(dir),
stderr: "pipe",
});

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

// stderr carries the crash report when the child dies; surface it in the
// failure diff but don't assert on it (debug builds emit benign noise).
if (exitCode !== 0) console.error(stderr);
expect(stdout.trim()).toBe(JSON.stringify(Array.from({ length: 16 }, () => 256)));
expect(exitCode).toBe(0);
});

test("BuildMessage finalize frees with the same allocator it was created with", async () => {
// BuildMessage.create() clones the message with the passed allocator
// but finalize() was freeing it with bun.default_allocator and never
Expand Down
61 changes: 61 additions & 0 deletions test/js/bun/transpiler/transpiler-error-gc-uaf.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Log.to_js (used by Bun.Transpiler().transform/transformSync when rejecting
// with parse errors, and by the module loader via process_fetch_log) builds
// an AggregateError by allocating one BuildMessage JS cell per log entry. The
// Rust port collected those cells in a heap Vec<JSValue>, which the
// conservative GC scan does not see, so an earlier cell could be swept while
// allocating a later one and the AggregateError would reference a zapped
// StructureID.
//
// useZombieMode scribbles 0xbadbeef0 over swept cells so the dangling access
// manifests deterministically; collectContinuously races the collector against
// the allocation loop so it reliably sweeps mid-loop.
import { expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";

const fixture = `
const src = Array.from({ length: 300 }, () => "a b").join("\\n");
const t = new Bun.Transpiler();
for (let i = 0; i < 20; i++) {
let err;
try { t.transformSync(src); } catch (e) { err = e; }
if (!(err instanceof AggregateError)) throw new Error("not AggregateError: " + err);
if (err.errors.length !== 256) throw new Error("wrong count: " + err.errors.length);
for (const m of err.errors) {
const msg = m.message;
if (msg !== 'Expected ";" but found "b"') {
throw new Error("corrupt BuildMessage: " + JSON.stringify(typeof msg) + " " + String(msg).slice(0, 80));
}
}
}
console.log("OK");
`;

test("Log.to_js roots BuildMessage cells across allocation", async () => {
using dir = tempDir("log-to-js-gc-root", {
"fixture.js": fixture,
});

// Windows + collectContinuously is prohibitively slow in CI and the code
// path is platform-agnostic, so rely on zombie mode alone there.
const gcEnv: Record<string, string | undefined> = {
...bunEnv,
BUN_JSC_useZombieMode: "1",
};
if (!isWindows) gcEnv.BUN_JSC_collectContinuously = "1";

await using proc = Bun.spawn({
cmd: [bunExe(), "fixture.js"],
env: gcEnv,
cwd: String(dir),
stderr: "pipe",
stdout: "pipe",
});

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

if (exitCode !== 0) {
expect(stderr).toBe("");
}
expect(stdout.trim()).toBe("OK");
expect(exitCode).toBe(0);
}, 60_000);
Loading