From 539f028d994ddca99d7a8a3636f0cc3a9216e687 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 18 Aug 2026 06:29:27 +0000 Subject: [PATCH 1/4] standalone: check the enum bytes of each module record before reading it from_bytes read every 52-byte CompiledModuleGraphFile record out of the executable with read_unaligned, which also turned its four repr(u8) enum bytes (encoding, loader, module_format, side) into enum values. A byte outside an enum's discriminants is not a value of that enum, so a damaged executable produced undefined behavior before any of the offset checks ran; in practice it ran as if the byte were valid. Decode each record field by field instead: the StringPointers are read at their offsets and the four enum bytes go through from_repr, returning a Corruption naming the field for a byte that is not a discriminant, so the executable exits with "Corrupted module graph: module is out of range" like the other damaged fields of the record. --- src/ast/loader.rs | 1 + src/standalone_graph/StandaloneModuleGraph.rs | 65 +++++++++++++------ src/standalone_graph/error.rs | 11 +++- test/bundler/bun-build-compile.test.ts | 20 +++++- 4 files changed, 74 insertions(+), 23 deletions(-) diff --git a/src/ast/loader.rs b/src/ast/loader.rs index fbb7dcbb46b8..9daf5a1398df 100644 --- a/src/ast/loader.rs +++ b/src/ast/loader.rs @@ -21,6 +21,7 @@ use enum_map::Enum; Debug, Hash, Enum, + strum::FromRepr, strum::IntoStaticStr, strum::VariantNames, )] diff --git a/src/standalone_graph/StandaloneModuleGraph.rs b/src/standalone_graph/StandaloneModuleGraph.rs index e9fa60fedd28..38f6775ab684 100644 --- a/src/standalone_graph/StandaloneModuleGraph.rs +++ b/src/standalone_graph/StandaloneModuleGraph.rs @@ -2,7 +2,7 @@ //! But this incurred a fixed 350ms overhead on every build, which is unacceptable //! so we give up on codesigning support on macOS for now until we can find a better solution -use core::mem::size_of; +use core::mem::{offset_of, size_of}; use core::ptr::NonNull; use std::io::Write as _; use std::sync::Arc; @@ -337,8 +337,41 @@ pub(crate) struct CompiledModuleGraphFile { pub side: FileSide, } +impl CompiledModuleGraphFile { + /// Decodes one record as `to_bytes` laid it out. The bytes come from the + /// executable, so the four enum fields are checked on the way in: reading the + /// record straight into a `CompiledModuleGraphFile` would turn a byte outside + /// an enum's discriminants into a value the enum cannot hold, which is + /// undefined behavior before anything looks at it. + fn read(record: &[u8; size_of::()]) -> crate::Result { + let pointer = |field: usize| -> StringPointer { + // SAFETY: `field` is the offset of a `StringPointer` field of `Self`, so + // it lies inside `record`, which holds a whole `Self`; every bit pattern + // is a valid `StringPointer` (two `u32`s), and `read_unaligned` does not + // need `record` to be 4-byte aligned. + unsafe { core::ptr::read_unaligned(record.as_ptr().add(field).cast::()) } + }; + Ok(Self { + name: pointer(offset_of!(Self, name)), + contents: pointer(offset_of!(Self, contents)), + sourcemap: pointer(offset_of!(Self, sourcemap)), + bytecode: pointer(offset_of!(Self, bytecode)), + module_info: pointer(offset_of!(Self, module_info)), + bytecode_origin_path: pointer(offset_of!(Self, bytecode_origin_path)), + encoding: Encoding::from_repr(record[offset_of!(Self, encoding)]) + .ok_or(Corruption::ModuleEncoding)?, + loader: Loader::from_repr(record[offset_of!(Self, loader)]) + .ok_or(Corruption::ModuleLoader)?, + module_format: ModuleFormat::from_repr(record[offset_of!(Self, module_format)]) + .ok_or(Corruption::ModuleFormat)?, + side: FileSide::from_repr(record[offset_of!(Self, side)]) + .ok_or(Corruption::ModuleSide)?, + }) + } +} + #[repr(u8)] -#[derive(Clone, Copy, PartialEq, Eq, Default)] +#[derive(Clone, Copy, PartialEq, Eq, Default, strum::FromRepr)] pub enum FileSide { #[default] Server = 0, @@ -346,7 +379,7 @@ pub enum FileSide { } #[repr(u8)] -#[derive(Clone, Copy, PartialEq, Eq, Default)] +#[derive(Clone, Copy, PartialEq, Eq, Default, strum::FromRepr)] pub enum Encoding { Binary = 0, #[default] @@ -356,7 +389,7 @@ pub enum Encoding { } #[repr(u8)] -#[derive(Clone, Copy, PartialEq, Eq, Default)] +#[derive(Clone, Copy, PartialEq, Eq, Default, strum::FromRepr)] pub enum ModuleFormat { #[default] None = 0, @@ -677,30 +710,22 @@ impl StandaloneModuleGraph { }; // Note: the modules blob sits at an arbitrary byte offset in the section, and // `&[CompiledModuleGraphFile]` would require natural alignment (StringPointer's u32 fields - // → 4-byte). We instead iterate by index and `read_unaligned` each fixed-size record into a - // local (`CompiledModuleGraphFile` is `Copy`/POD), so no `&T` ever points at unaligned memory. - let modules_list_count = modules_list_bytes.len() / size_of::(); - let modules_list_base = modules_list_bytes.as_ptr(); + // → 4-byte). We instead split it into fixed-size byte records and decode each one with + // `CompiledModuleGraphFile::read`, so no `&T` ever points at unaligned memory. + let (records, _) = + modules_list_bytes.as_chunks::<{ size_of::() }>(); // `entry_point()` indexes `files` by this id. `to_bytes` writes one file per // record with distinct names, and a repeated name is rejected below, so the // record count is the file count. - if offsets.entry_point_id as usize >= modules_list_count { + if offsets.entry_point_id as usize >= records.len() { return Err(Corruption::EntryPointId.into()); } let mut modules = StringArrayHashMap::::new(); - modules.reserve(modules_list_count); - for i in 0..modules_list_count { - // SAFETY: index < count derived from byte length above; bytes live for 'static. - let module: CompiledModuleGraphFile = unsafe { - core::ptr::read_unaligned( - modules_list_base - .add(i * size_of::()) - .cast::(), - ) - }; - let module = &module; + modules.reserve(records.len()); + for record in records { + let module = CompiledModuleGraphFile::read(record)?; // SAFETY: each name/contents/sourcemap/bytecode_origin_path subrange is checked // against `raw_len` and is disjoint from the writable bytecode/module_info // subranges (serialized by `to_bytes`); section bytes are a live 'static allocation. diff --git a/src/standalone_graph/error.rs b/src/standalone_graph/error.rs index 4267f727ee16..1538359f373c 100644 --- a/src/standalone_graph/error.rs +++ b/src/standalone_graph/error.rs @@ -1,6 +1,7 @@ /// What `StandaloneModuleGraph::from_bytes` found wrong with the graph embedded /// in the executable. Every offset it reads comes from the file, so each one is -/// checked before it is dereferenced. +/// checked before it is dereferenced, and every enum byte is checked before it +/// becomes an enum. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Corruption { ByteCount, @@ -13,6 +14,10 @@ pub enum Corruption { ModuleBytecode, ModuleInfo, BytecodeOriginPath, + ModuleEncoding, + ModuleLoader, + ModuleFormat, + ModuleSide, CompileExecArgv, } @@ -33,6 +38,10 @@ impl Corruption { Self::BytecodeOriginPath => { "Corrupted module graph: bytecode origin path is out of range" } + Self::ModuleEncoding => "Corrupted module graph: module encoding is out of range", + Self::ModuleLoader => "Corrupted module graph: module loader is out of range", + Self::ModuleFormat => "Corrupted module graph: module format is out of range", + Self::ModuleSide => "Corrupted module graph: module side is out of range", Self::CompileExecArgv => "Corrupted module graph: compile exec argv is out of range", } } diff --git a/test/bundler/bun-build-compile.test.ts b/test/bundler/bun-build-compile.test.ts index 9b8c22be2e85..22b6b6eced82 100644 --- a/test/bundler/bun-build-compile.test.ts +++ b/test/bundler/bun-build-compile.test.ts @@ -150,7 +150,8 @@ describe("compiled binary validity", () => { // The embedded module graph ends with `Offsets` (repr(C): byte_count u64, modules_ptr {offset u32, length u32}, // entry_point_id u32, compile_exec_argv_ptr, flags) followed by the trailer. `byte_count` is the size of the // graph before `Offsets`, and `modules_ptr` points into it at the 52-byte `CompiledModuleGraphFile` records, - // each of which starts with the module name as {offset u32, length u32} into the graph. + // each of which starts with the module name as {offset u32, length u32} into the graph and ends with four + // one-byte enums: encoding, loader, module format, side. const GRAPH_TRAILER = "\n---- Bun! ----\n"; const OFFSETS_SIZE = 32; const MODULE_RECORD_SIZE = 52; @@ -168,11 +169,12 @@ describe("compiled binary validity", () => { } type ModuleGraphLayout = ReturnType; + const oneFile = { "app.js": `console.log("should not run");` }; const twoFiles = { "app.js": `console.log("should not run");`, "worker.js": `console.log("should not run");` }; const corruptedGraphs = [ { label: "entry_point_id equal to the module count", - files: { "app.js": `console.log("should not run");` }, + files: oneFile, error: "entry point ID is out of range for the module list", corrupt(executable: Buffer, { moduleRecordCount, entryPointIdAt }: ModuleGraphLayout) { executable.writeUInt32LE(moduleRecordCount, entryPointIdAt); @@ -205,6 +207,20 @@ describe("compiled binary validity", () => { executable.writeUInt32LE(0x7fffffff, modulesAt + MODULE_RECORD_SIZE + 12); }, }, + // The enum bytes at the end of the first record. 0xff is not a value of any of the four enums. + ...[ + { field: "encoding", at: 48 }, + { field: "loader", at: 49 }, + { field: "format", at: 50 }, + { field: "side", at: 51 }, + ].map(({ field, at }) => ({ + label: `an out-of-range module ${field} byte`, + files: oneFile, + error: `module ${field} is out of range`, + corrupt(executable: Buffer, { modulesAt }: ModuleGraphLayout) { + executable[modulesAt + at] = 0xff; + }, + })), ]; // A damaged graph must surface as an error, not an out-of-bounds panic in From 1ddb0d35403e43636d23bc6d23b01585b2d7ebd0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 18 Aug 2026 14:14:42 +0000 Subject: [PATCH 2/4] standalone: unit test CompiledModuleGraphFile::read and run the crate under Miri Two tests in the crate: a record with a distinct value in every field round-trips through read, and every byte value in each enum slot is accepted exactly when it is a discriminant of that slot's enum, with the Corruption naming the slot otherwise. The second test also asserts that the enum slots are the whole tail of the record, so a field added later needs an entry. bun_standalone_graph joins the Miri crate set so the sweep runs under Miri, which reports the old whole-record read as UB at the first byte outside an enum. The FromRepr derive on Loader moves after VariantNames, the position another open change adds it in, so either landing order merges to one derive. --- scripts/rust-miri.ts | 1 + src/ast/loader.rs | 2 +- src/standalone_graph/StandaloneModuleGraph.rs | 97 ++++++++++++++++++- 3 files changed, 95 insertions(+), 5 deletions(-) diff --git a/scripts/rust-miri.ts b/scripts/rust-miri.ts index 09d5892297b3..992655968259 100644 --- a/scripts/rust-miri.ts +++ b/scripts/rust-miri.ts @@ -50,6 +50,7 @@ const MIRI_CRATES = [ "bun_ptr", "bun_resolve_builtins", "bun_shell_parser", + "bun_standalone_graph", "bun_threading", "bun_wyhash", ]; diff --git a/src/ast/loader.rs b/src/ast/loader.rs index 9daf5a1398df..5f46b7fd8d40 100644 --- a/src/ast/loader.rs +++ b/src/ast/loader.rs @@ -21,9 +21,9 @@ use enum_map::Enum; Debug, Hash, Enum, - strum::FromRepr, strum::IntoStaticStr, strum::VariantNames, + strum::FromRepr, )] // The lower_snake names are exposed to JS (HTMLImportManifest // `"loader":`, BuildArtifact.loader) so the strum serialization must match exactly. diff --git a/src/standalone_graph/StandaloneModuleGraph.rs b/src/standalone_graph/StandaloneModuleGraph.rs index 38f6775ab684..85e78fceda70 100644 --- a/src/standalone_graph/StandaloneModuleGraph.rs +++ b/src/standalone_graph/StandaloneModuleGraph.rs @@ -321,7 +321,7 @@ impl bun_resolver::StandaloneModuleGraph for StandaloneModuleGraph { } #[repr(C)] -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) struct CompiledModuleGraphFile { pub name: StringPointer, pub contents: StringPointer, @@ -371,7 +371,7 @@ impl CompiledModuleGraphFile { } #[repr(u8)] -#[derive(Clone, Copy, PartialEq, Eq, Default, strum::FromRepr)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default, strum::FromRepr)] pub enum FileSide { #[default] Server = 0, @@ -379,7 +379,7 @@ pub enum FileSide { } #[repr(u8)] -#[derive(Clone, Copy, PartialEq, Eq, Default, strum::FromRepr)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default, strum::FromRepr)] pub enum Encoding { Binary = 0, #[default] @@ -389,7 +389,7 @@ pub enum Encoding { } #[repr(u8)] -#[derive(Clone, Copy, PartialEq, Eq, Default, strum::FromRepr)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default, strum::FromRepr)] pub enum ModuleFormat { #[default] None = 0, @@ -2624,3 +2624,92 @@ pub(crate) fn serialize_json_source_map_for_standalone( debug_assert!(header_list.len() == string_payload_start_location); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + /// A record with a different value in every field and no enum left at its + /// default, plus its bytes as `to_bytes` serializes them. + fn record() -> ( + CompiledModuleGraphFile, + [u8; size_of::()], + ) { + let pointer = |offset, length| StringPointer { offset, length }; + let module = CompiledModuleGraphFile { + name: pointer(1, 2), + contents: pointer(3, 4), + sourcemap: pointer(5, 6), + bytecode: pointer(7, 8), + module_info: pointer(9, 10), + bytecode_origin_path: pointer(11, 12), + encoding: Encoding::Utf8, + loader: Loader::Xml, + module_format: ModuleFormat::Cjs, + side: FileSide::Client, + }; + // SAFETY: `CompiledModuleGraphFile` is `repr(C)` without padding (six 8-byte + // pointers and then four bytes, 4-byte aligned), so every byte of it is + // initialized, and a byte array has no alignment requirement. + let bytes = unsafe { + core::ptr::from_ref(&module) + .cast::<[u8; size_of::()]>() + .read() + }; + (module, bytes) + } + + #[test] + fn read_decodes_what_to_bytes_writes() { + let (module, bytes) = record(); + assert_eq!(CompiledModuleGraphFile::read(&bytes), Ok(module)); + } + + #[test] + fn read_checks_each_enum_byte_against_its_own_enum() { + let (_, valid) = record(); + let enum_bytes: [(usize, fn(u8) -> bool, Corruption); 4] = [ + ( + offset_of!(CompiledModuleGraphFile, encoding), + |byte| Encoding::from_repr(byte).is_some(), + Corruption::ModuleEncoding, + ), + ( + offset_of!(CompiledModuleGraphFile, loader), + |byte| Loader::from_repr(byte).is_some(), + Corruption::ModuleLoader, + ), + ( + offset_of!(CompiledModuleGraphFile, module_format), + |byte| ModuleFormat::from_repr(byte).is_some(), + Corruption::ModuleFormat, + ), + ( + offset_of!(CompiledModuleGraphFile, side), + |byte| FileSide::from_repr(byte).is_some(), + Corruption::ModuleSide, + ), + ]; + // The enums are the whole tail of the record, so a new one needs an entry above. + assert_eq!( + enum_bytes.len(), + size_of::() - offset_of!(CompiledModuleGraphFile, encoding) + ); + for (at, is_discriminant, what) in enum_bytes { + for byte in 0..=u8::MAX { + let mut record = valid; + record[at] = byte; + let expected: crate::Result<()> = if is_discriminant(byte) { + Ok(()) + } else { + Err(what.into()) + }; + assert_eq!( + CompiledModuleGraphFile::read(&record).map(|_| ()), + expected, + "byte {byte} at offset {at}" + ); + } + } + } +} From b1dcc2fce858d73d17dbb1c808e72363fef7ec06 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 18 Aug 2026 14:19:08 +0000 Subject: [PATCH 3/4] standalone: shorten the comments around the record decoder --- src/standalone_graph/StandaloneModuleGraph.rs | 18 +++++++----------- src/standalone_graph/error.rs | 5 ++--- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/standalone_graph/StandaloneModuleGraph.rs b/src/standalone_graph/StandaloneModuleGraph.rs index 85e78fceda70..b949511df85f 100644 --- a/src/standalone_graph/StandaloneModuleGraph.rs +++ b/src/standalone_graph/StandaloneModuleGraph.rs @@ -338,11 +338,9 @@ pub(crate) struct CompiledModuleGraphFile { } impl CompiledModuleGraphFile { - /// Decodes one record as `to_bytes` laid it out. The bytes come from the - /// executable, so the four enum fields are checked on the way in: reading the - /// record straight into a `CompiledModuleGraphFile` would turn a byte outside - /// an enum's discriminants into a value the enum cannot hold, which is - /// undefined behavior before anything looks at it. + /// Decodes one record as `to_bytes` laid it out. The record is not read as a + /// whole because a byte outside one of its enums would make that read + /// undefined behavior, so each enum byte is checked first. fn read(record: &[u8; size_of::()]) -> crate::Result { let pointer = |field: usize| -> StringPointer { // SAFETY: `field` is the offset of a `StringPointer` field of `Self`, so @@ -708,10 +706,9 @@ impl StandaloneModuleGraph { Corruption::ModuleList, )? }; - // Note: the modules blob sits at an arbitrary byte offset in the section, and - // `&[CompiledModuleGraphFile]` would require natural alignment (StringPointer's u32 fields - // → 4-byte). We instead split it into fixed-size byte records and decode each one with - // `CompiledModuleGraphFile::read`, so no `&T` ever points at unaligned memory. + // The blob sits at an arbitrary byte offset, so it is split into byte records for + // `CompiledModuleGraphFile::read` rather than viewed as a `&[CompiledModuleGraphFile]`, + // which would need 4-byte alignment. let (records, _) = modules_list_bytes.as_chunks::<{ size_of::() }>(); @@ -2629,8 +2626,7 @@ pub(crate) fn serialize_json_source_map_for_standalone( mod tests { use super::*; - /// A record with a different value in every field and no enum left at its - /// default, plus its bytes as `to_bytes` serializes them. + /// A record with a distinct value in every field, and its bytes as `to_bytes` writes them. fn record() -> ( CompiledModuleGraphFile, [u8; size_of::()], diff --git a/src/standalone_graph/error.rs b/src/standalone_graph/error.rs index 1538359f373c..ed46d848e752 100644 --- a/src/standalone_graph/error.rs +++ b/src/standalone_graph/error.rs @@ -1,7 +1,6 @@ /// What `StandaloneModuleGraph::from_bytes` found wrong with the graph embedded -/// in the executable. Every offset it reads comes from the file, so each one is -/// checked before it is dereferenced, and every enum byte is checked before it -/// becomes an enum. +/// in the executable. Every offset and enum byte it reads comes from the file, +/// so each one is checked before it is used. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Corruption { ByteCount, From f1d517e4284ff22e3ae4a8107c5a1c82ceb600b0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 18 Aug 2026 14:24:02 +0000 Subject: [PATCH 4/4] standalone: trim the decoder comments and leave the Corruption doc as it was --- src/standalone_graph/StandaloneModuleGraph.rs | 10 ++++------ src/standalone_graph/error.rs | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/standalone_graph/StandaloneModuleGraph.rs b/src/standalone_graph/StandaloneModuleGraph.rs index b949511df85f..0756d88eeb00 100644 --- a/src/standalone_graph/StandaloneModuleGraph.rs +++ b/src/standalone_graph/StandaloneModuleGraph.rs @@ -338,9 +338,8 @@ pub(crate) struct CompiledModuleGraphFile { } impl CompiledModuleGraphFile { - /// Decodes one record as `to_bytes` laid it out. The record is not read as a - /// whole because a byte outside one of its enums would make that read - /// undefined behavior, so each enum byte is checked first. + /// Decodes one record as `to_bytes` laid it out. Reading the record as a whole is + /// undefined behavior if a byte is outside its enum, so each enum byte is checked first. fn read(record: &[u8; size_of::()]) -> crate::Result { let pointer = |field: usize| -> StringPointer { // SAFETY: `field` is the offset of a `StringPointer` field of `Self`, so @@ -706,9 +705,8 @@ impl StandaloneModuleGraph { Corruption::ModuleList, )? }; - // The blob sits at an arbitrary byte offset, so it is split into byte records for - // `CompiledModuleGraphFile::read` rather than viewed as a `&[CompiledModuleGraphFile]`, - // which would need 4-byte alignment. + // Byte records, not a `&[CompiledModuleGraphFile]`: the blob sits at an arbitrary byte + // offset and that slice would need 4-byte alignment. let (records, _) = modules_list_bytes.as_chunks::<{ size_of::() }>(); diff --git a/src/standalone_graph/error.rs b/src/standalone_graph/error.rs index ed46d848e752..314c1124566d 100644 --- a/src/standalone_graph/error.rs +++ b/src/standalone_graph/error.rs @@ -1,6 +1,6 @@ /// What `StandaloneModuleGraph::from_bytes` found wrong with the graph embedded -/// in the executable. Every offset and enum byte it reads comes from the file, -/// so each one is checked before it is used. +/// in the executable. Every offset it reads comes from the file, so each one is +/// checked before it is dereferenced. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Corruption { ByteCount,