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
47 changes: 26 additions & 21 deletions src/exe_format/macho.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -262,8 +268,8 @@ impl MachoFile {
let seg_sz = size_of::<macho::segment_command_64>();
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);
}

Expand All @@ -285,8 +291,7 @@ impl MachoFile {
let seg_sz = size_of::<macho::segment_command_64>();

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 =
Expand Down Expand Up @@ -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,
Expand Down
38 changes: 23 additions & 15 deletions test/bundler/bundler_compile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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(),
Expand All @@ -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.
Expand Down
Loading