From 648fbd7bde3a683a010646bab5bee0a2778da155 Mon Sep 17 00:00:00 2001 From: GGWA <65288322+rech4210@users.noreply.github.com> Date: Mon, 2 Nov 2026 12:09:52 +0900 Subject: [PATCH] A Mach-O method list at the end of the address space is read without overflowing (#205) Each method entry's address and its fields' addresses are computed with checked arithmetic, so a method list near the end of the address space ends the list instead of panicking (debug) or reading a wrapped address (release). --- crates/formats/src/macho.rs | 32 ++++++++++++++++++++++++++++---- docs/ROADMAP.md | 5 +++-- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/crates/formats/src/macho.rs b/crates/formats/src/macho.rs index ad48250f..39da3522 100644 --- a/crates/formats/src/macho.rs +++ b/crates/formats/src/macho.rs @@ -472,11 +472,11 @@ fn method_list(data: &[u8], b: &Binary, list: u64, kind: &'static str, out: &mut return; } for i in 0..u64::from(count).min(MAX_METHODS as u64) { - let e = list + 8 + i * entsize; + let Some(e) = i.checked_mul(entsize).and_then(|offset| list.checked_add(8)?.checked_add(offset)) else { break }; if relative { let Some(raw) = b.read(data, e, 12) else { break }; let rr = Reader::new(raw); - let rel = |o: usize| rr.u32_le(o).map(|v| (e + o as u64).wrapping_add(i64::from(v as i32) as u64)); + let rel = |o: usize| rr.u32_le(o).and_then(|v| e.checked_add(o as u64).map(|a| a.wrapping_add(i64::from(v as i32) as u64))); let name_field = rel(0); // The name offset points at a selector reference unless selectors are direct. let selector = if ent_flags & 0x4000_0000 != 0 { @@ -488,8 +488,9 @@ fn method_list(data: &[u8], b: &Binary, list: u64, kind: &'static str, out: &mut out.push(ObjcMethod { selector, kind, imp: rel(8).filter(|a| b.is_mapped(*a)), types: rel(4).and_then(|a| cstr_at(data, b, a)) }); } else { let Some(selector) = ptr_at(data, b, e).and_then(|a| cstr_at(data, b, a)) else { continue }; - let types = ptr_at(data, b, e + 8).and_then(|a| cstr_at(data, b, a)); - out.push(ObjcMethod { selector, kind, imp: ptr_at(data, b, e + 16), types }); + let (Some(types_field), Some(imp_field)) = (e.checked_add(8), e.checked_add(16)) else { continue }; + let types = ptr_at(data, b, types_field).and_then(|a| cstr_at(data, b, a)); + out.push(ObjcMethod { selector, kind, imp: ptr_at(data, b, imp_field), types }); } } } @@ -609,6 +610,29 @@ mod tests { assert_eq!(meta.protocols[0].name, tk::OBJC_PROTOCOL); } + #[test] + fn objc_method_list_address_overflow_never_panics() { + let mut m = tk::macho64(); + let list = u64::MAX - 8; + let fileoff = m.len() as u64; + // Reuse the sectionless __PAGEZERO command for an eight-byte, file-backed segment. + m[32 + 24..32 + 32].copy_from_slice(&list.to_le_bytes()); + m[32 + 32..32 + 40].copy_from_slice(&8u64.to_le_bytes()); + m[32 + 40..32 + 48].copy_from_slice(&fileoff.to_le_bytes()); + m[32 + 48..32 + 56].copy_from_slice(&8u64.to_le_bytes()); + m.extend_from_slice(&24u32.to_le_bytes()); + m.extend_from_slice(&2u32.to_le_bytes()); + m[0x1060..0x1068].copy_from_slice(&list.to_le_bytes()); // class_ro.baseMethods + + let b = crate::parse_binary(&m).unwrap(); + assert!(b.is_mapped(list)); + assert_eq!(ptr_at(&m, &b, tk::MACHO_BASE + 0x1060), Some(list)); + assert_eq!(b.read(&m, list, 8).unwrap().len(), 8); + let meta = objc_metadata(&m, &b); + assert_eq!(meta.classes.len(), 1); + assert!(meta.classes[0].methods.is_empty()); + } + #[test] fn fat_slices_and_parse() { let thin = tk::macho64(); diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 3dcf3f2b..2ef71899 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -168,8 +168,9 @@ Owner: open · Status: done (branch chore/r0.5-cleanups) - [x] rustfmt drift on main (11 files) - [x] Checked arithmetic on file offsets in `fsread/extfs.rs`: an inode offset near `usize::MAX` overflows `o + 128` (two sites), found by a lint scan of `crates/formats` (#42) -- [ ] `macho.rs` `method_list`: `list + 8 + i * entsize` adds to a pointer read from the file, so a - segment that ends at `u64::MAX` overflows it (a lint scan flagged it; not reproduced yet) +- [x] `macho.rs` `method_list`: `list + 8 + i * entsize` adds to a pointer read from the file, so a + segment that ends at `u64::MAX` overflowed it (found by a lint scan; now reproduced by a + regression test, and the entry addresses are checked) ### R0.8 Tests never touch the real machine Owner: open · Status: done, except protecting `main` (a maintainer setting)