diff --git a/src/exe_format/macho.rs b/src/exe_format/macho.rs index 129a9bf0b9d1..bf7ecbbc9ac4 100644 --- a/src/exe_format/macho.rs +++ b/src/exe_format/macho.rs @@ -182,20 +182,36 @@ impl MachoFile { return Err(MachoError::InvalidObject); } - // Calculate how much larger/smaller the section will be compared to its current size - let size_diff: i64 = i64::try_from(aligned_size).expect("int cast") - - i64::try_from(original_segsize).expect("int cast"); + // Validate the template's __BUN segment lies inside the file before any + // growth arithmetic: these offsets came from untrusted load commands. + let prev_len = self.data.len(); + let original_bun_end = usize::try_from(original_fileoff) + .ok() + .and_then(|off| off.checked_add(usize::try_from(original_segsize).ok()?)) + .ok_or(MachoError::OffsetOverflow)?; + let original_data_end = + usize::try_from(original_data_end).map_err(|_| MachoError::OffsetOverflow)?; + if original_bun_end > prev_len || original_data_end > original_bun_end { + return Err(MachoError::OffsetOutOfRange); + } + + // __BUN is grown to fit the bundle; shrinking is not implemented (the + // offset-shift logic below only moves data forward). Real Bun templates + // ship a minimal placeholder so `aligned_size >= filesize` always holds. + let size_diff: u64 = aligned_size + .checked_sub(original_segsize) + .ok_or(MachoError::InvalidObject)?; // We assume that the section is page-aligned, so we can calculate the number of new pages - debug_assert!(size_diff % PAGE_SIZE as i64 == 0); - let num_of_new_pages = size_diff / PAGE_SIZE as i64; + debug_assert!(size_diff.is_multiple_of(PAGE_SIZE)); + let num_of_new_pages = size_diff / PAGE_SIZE; // Pre-grow the backing buffer to fit: the `size_diff` bytes of new section // content and one SHA-256 hash per new page. `buildAndSign` may grow further // to write the complete signature, but reserving this up front avoids the // common reallocation. self.data.reserve( - usize::try_from(size_diff + num_of_new_pages * HASH_SIZE as i64).expect("int cast"), + usize::try_from(size_diff + num_of_new_pages * HASH_SIZE as u64).expect("int cast"), ); let linkedit_seg_idx = match linkedit_seg_idx { @@ -205,16 +221,6 @@ impl MachoFile { let mut sig_size: usize = 0; - let prev_len = self.data.len(); - let original_bun_end = usize::try_from(original_fileoff) - .ok() - .and_then(|off| off.checked_add(usize::try_from(original_segsize).ok()?)) - .ok_or(MachoError::OffsetOverflow)?; - let original_data_end = - usize::try_from(original_data_end).map_err(|_| MachoError::OffsetOverflow)?; - if original_bun_end > prev_len || original_data_end > original_bun_end { - return Err(MachoError::OffsetOutOfRange); - } // SAFETY: we just reserved `size_diff` bytes; new_len <= capacity. The newly-exposed bytes // are written below before being read (memmove + memset cover the whole range). unsafe { @@ -262,8 +268,8 @@ impl MachoFile { let seg_sz = size_of::(); let mut v: macho::segment_command_64 = read_struct(&self.data[linkedit_seg_idx..][..seg_sz]); - v.fileoff += usize::try_from(size_diff).expect("int cast") as u64; - v.vmaddr += usize::try_from(size_diff).expect("int cast") as u64; + v.fileoff += size_diff; + v.vmaddr += size_diff; write_struct(&mut self.data[linkedit_seg_idx..][..seg_sz], &v); } @@ -285,8 +291,7 @@ impl MachoFile { let seg_sz = size_of::(); let mut cs: macho::linkedit_data_command = read_struct(&self.data[idx..][..cs_sz]); - let new_sig_dataoff: u64 = - cs.dataoff as u64 + u64::try_from(size_diff).expect("int cast"); + let new_sig_dataoff: u64 = cs.dataoff as u64 + size_diff; let new_sig_size = MachoSigner::compute_signature_size(new_sig_dataoff); let mut seg: macho::segment_command_64 = @@ -316,7 +321,7 @@ impl MachoFile { let (le_fileoff, le_filesize) = (seg.fileoff, seg.filesize); self.update_load_command_offsets( original_fileoff, - u64::try_from(size_diff).expect("int cast"), + size_diff, le_fileoff, le_filesize, sig_size, diff --git a/test/bundler/bundler_compile.test.ts b/test/bundler/bundler_compile.test.ts index b370a9889487..29bde1b2987e 100644 --- a/test/bundler/bundler_compile.test.ts +++ b/test/bundler/bundler_compile.test.ts @@ -1102,9 +1102,9 @@ test("compile --compile-executable-path rejects a Mach-O template whose __BUN se const LC_SEGMENT_64 = 0x19; // Minimal Mach-O "base executable": a __BUN segment with one __bun section followed by a - // __LINKEDIT segment. `bunFileOff` is where the load commands claim the __BUN data lives. - function machoTemplate(bunFileOff: number): Buffer { - const fileSize = 0x8100; // 33 KB of actual bytes + // __LINKEDIT segment. `bunFileOff`/`bunFileSize` are where the load commands claim the + // __BUN data lives; `fileSize` is how many bytes the template actually contains. + function machoTemplate(bunFileOff: number, bunFileSize = 0x4000, fileSize = 0x8100): Buffer { const segCmdSize = 72; // sizeof(segment_command_64) const sectSize = 80; // sizeof(section_64) const sizeofcmds = segCmdSize + sectSize + segCmdSize; @@ -1125,9 +1125,9 @@ test("compile --compile-executable-path rejects a Mach-O template whose __BUN se buf.writeUInt32LE(segCmdSize + sectSize, o + 4); // cmdsize writeName(o + 8, "__BUN"); buf.writeBigUInt64LE(0x1_0000_4000n, o + 24); // vmaddr - buf.writeBigUInt64LE(0x4000n, o + 32); // vmsize + buf.writeBigUInt64LE(BigInt(bunFileSize), o + 32); // vmsize buf.writeBigUInt64LE(BigInt(bunFileOff), o + 40); // fileoff - buf.writeBigUInt64LE(0x4000n, o + 48); // filesize + buf.writeBigUInt64LE(BigInt(bunFileSize), o + 48); // filesize buf.writeInt32LE(7, o + 56); // maxprot buf.writeInt32LE(3, o + 60); // initprot buf.writeUInt32LE(1, o + 64); // nsects @@ -1137,7 +1137,7 @@ test("compile --compile-executable-path rejects a Mach-O template whose __BUN se writeName(o, "__bun"); writeName(o + 16, "__BUN"); buf.writeBigUInt64LE(0x1_0000_4000n, o + 32); // addr - buf.writeBigUInt64LE(0x4000n, o + 40); // size + buf.writeBigUInt64LE(BigInt(bunFileSize), o + 40); // size buf.writeUInt32LE(bunFileOff, o + 48); // offset buf.writeUInt32LE(14, o + 52); // align = 2^14 @@ -1146,9 +1146,9 @@ test("compile --compile-executable-path rejects a Mach-O template whose __BUN se buf.writeUInt32LE(LC_SEGMENT_64, o); buf.writeUInt32LE(segCmdSize, o + 4); writeName(o + 8, "__LINKEDIT"); - buf.writeBigUInt64LE(0x1_0000_8000n, o + 24); // vmaddr + buf.writeBigUInt64LE(0x1_0001_0000n, o + 24); // vmaddr buf.writeBigUInt64LE(0x1000n, o + 32); // vmsize - buf.writeBigUInt64LE(BigInt(bunFileOff + 0x4000), o + 40); // fileoff (right after __BUN) + buf.writeBigUInt64LE(BigInt(bunFileOff + bunFileSize), o + 40); // fileoff (right after __BUN) buf.writeBigUInt64LE(0x100n, o + 48); // filesize buf.writeInt32LE(1, o + 56); // maxprot buf.writeInt32LE(1, o + 60); // initprot @@ -1161,11 +1161,19 @@ test("compile --compile-executable-path rejects a Mach-O template whose __BUN se }); const cwd = String(dir); - // Template whose __BUN offsets point 1 GiB past the end of the 33 KB file. - const badTemplate = join(cwd, "template-bad"); - await Bun.write(badTemplate, machoTemplate(0x40000000)); - const outBad = join(cwd, "out-bad"); - { + for (const [name, bytes, wantErr] of [ + // __BUN fileoff points 1 GiB past the end of the 33 KB file. + ["fileoff-past-eof", machoTemplate(0x40000000), "OffsetOutOfRange"], + // __BUN filesize (32 KB) exceeds the 256-byte file: the bounds check must reject this + // before the growth `reserve()` (which would otherwise see a negative size_diff). + ["filesize-past-eof", machoTemplate(0, 0x8000, 256), "OffsetOutOfRange"], + // __BUN filesize (32 KB) is in-bounds but larger than the 16 KB aligned bundle slot; + // write_section only grows, so a template that would require shrinking is rejected. + ["filesize-needs-shrink", machoTemplate(0x4000, 0x8000, 0xc100), "InvalidObject"], + ] as const) { + const badTemplate = join(cwd, `template-${name}`); + await Bun.write(badTemplate, bytes); + const outBad = join(cwd, `out-${name}`); await using proc = Bun.spawn({ cmd: [ bunExe(), @@ -1184,8 +1192,8 @@ test("compile --compile-executable-path rejects a Mach-O template whose __BUN se stderr: "pipe", }); const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - // The out-of-range offsets must be reported as a clean error... - expect(stderr).toContain("OffsetOutOfRange"); + // The invalid template must be reported as a clean error... + expect({ name, stderr }).toEqual({ name, stderr: expect.stringContaining(wantErr) }); // ...no output executable is produced... expect(await Bun.file(outBad).exists()).toBe(false); // ...and the build exits with a normal failure code instead of crashing.