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 fbb7dcbb46b8..5f46b7fd8d40 100644 --- a/src/ast/loader.rs +++ b/src/ast/loader.rs @@ -23,6 +23,7 @@ use enum_map::Enum; Enum, 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 e9fa60fedd28..0756d88eeb00 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; @@ -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, @@ -337,8 +337,38 @@ pub(crate) struct CompiledModuleGraphFile { pub side: FileSide, } +impl CompiledModuleGraphFile { + /// 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 + // 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, Debug, PartialEq, Eq, Default, strum::FromRepr)] pub enum FileSide { #[default] Server = 0, @@ -346,7 +376,7 @@ pub enum FileSide { } #[repr(u8)] -#[derive(Clone, Copy, PartialEq, Eq, Default)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default, strum::FromRepr)] pub enum Encoding { Binary = 0, #[default] @@ -356,7 +386,7 @@ pub enum Encoding { } #[repr(u8)] -#[derive(Clone, Copy, PartialEq, Eq, Default)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default, strum::FromRepr)] pub enum ModuleFormat { #[default] None = 0, @@ -675,32 +705,22 @@ 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 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(); + // 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::() }>(); // `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. @@ -2599,3 +2619,91 @@ 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 distinct value in every field, and its bytes as `to_bytes` writes 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}" + ); + } + } + } +} diff --git a/src/standalone_graph/error.rs b/src/standalone_graph/error.rs index 4267f727ee16..314c1124566d 100644 --- a/src/standalone_graph/error.rs +++ b/src/standalone_graph/error.rs @@ -13,6 +13,10 @@ pub enum Corruption { ModuleBytecode, ModuleInfo, BytecodeOriginPath, + ModuleEncoding, + ModuleLoader, + ModuleFormat, + ModuleSide, CompileExecArgv, } @@ -33,6 +37,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