mirror of
https://github.com/storytold/craft-anything.git
synced 2026-11-05 20:47:45 +01:00
Skip an ext inode that starts at the end of the address space (#42)
A group descriptor could point the inode table so far out that an inode started within 128 bytes of usize::MAX; the two checks that an inode fits in the image added 128 unchecked, which panicked in debug builds and wrapped in release. They now use checked_add, so such a root inode is refused and such a child skipped. Two regression tests, and a roadmap line for the same pattern in macho.rs method_list.
This commit is contained in:
@@ -225,7 +225,7 @@ fn walk(data: &[u8], sb: &Sb, ino: u32, parent: &str, limits: &Limits, img: &mut
|
||||
seen.push(ino);
|
||||
let Some(inode_off) = inode_offset(data, sb, ino) else { return };
|
||||
let r = Reader::new(data);
|
||||
let io = usize::try_from(inode_off).ok().filter(|&o| o + 128 <= data.len());
|
||||
let io = usize::try_from(inode_off).ok().filter(|&o| o.checked_add(128).is_some_and(|end| end <= data.len()));
|
||||
let Some(io) = io else { return };
|
||||
let size = u64::from(r.u32_le(io + 4).unwrap_or(0)) | (u64::from(r.u32_le(io + 108).unwrap_or(0)) << 32);
|
||||
let flags = r.u32_le(io + 32).unwrap_or(0);
|
||||
@@ -263,7 +263,10 @@ fn walk(data: &[u8], sb: &Sb, ino: u32, parent: &str, limits: &Limits, img: &mut
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn child_entry(data: &[u8], sb: &Sb, ino: u32, ftype: u8, path: String, limits: &Limits, img: &mut FsImage, depth: usize, seen: &mut Vec<u32>) {
|
||||
// ftype: 1=file, 2=dir, 7=symlink (when the filetype feature is on); fall back to i_mode.
|
||||
let Some(io) = inode_offset(data, sb, ino).and_then(|o| usize::try_from(o).ok()).filter(|&o| o + 128 <= data.len()) else { return };
|
||||
let Some(io) = inode_offset(data, sb, ino).and_then(|o| usize::try_from(o).ok()).filter(|&o| o.checked_add(128).is_some_and(|end| end <= data.len()))
|
||||
else {
|
||||
return;
|
||||
};
|
||||
let r = Reader::new(data);
|
||||
let mode = r.u16_le(io).unwrap_or(0) & 0xf000;
|
||||
let is_dir = ftype == 2 || mode == 0x4000;
|
||||
@@ -328,6 +331,37 @@ mod tests {
|
||||
check(true);
|
||||
}
|
||||
|
||||
/// Switch to 64-bit group descriptors and send `group`'s inode table to the last
|
||||
/// 1 KiB block of the address space, so an inode there starts within 128 bytes of `usize::MAX`.
|
||||
fn point_table_at_the_end(img: &mut [u8], group: usize) {
|
||||
img[1024 + 96..1024 + 100].copy_from_slice(&0x80u32.to_le_bytes()); // incompat: 64-bit
|
||||
img[1024 + 0xfe..1024 + 0x100].copy_from_slice(&64u16.to_le_bytes()); // descriptor size
|
||||
let d = 2048 + group * 64;
|
||||
img[d + 8..d + 12].copy_from_slice(&u32::MAX.to_le_bytes());
|
||||
img[d + 40..d + 44].copy_from_slice(&0x003f_ffffu32.to_le_bytes());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn root_inode_at_the_end_of_the_address_space_is_refused() {
|
||||
let mut base = craft_anything_testkit::firmware::ext_image(false);
|
||||
base[1024 + 88..1024 + 90].copy_from_slice(&1000u16.to_le_bytes()); // inode size
|
||||
point_table_at_the_end(&mut base, 0);
|
||||
let img = read(&base, &Limits::default());
|
||||
assert!(img.entries.is_empty(), "{:?}", img.entries);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_inode_at_the_end_of_the_address_space_is_skipped() {
|
||||
let mut base = craft_anything_testkit::firmware::ext_image(false);
|
||||
point_table_at_the_end(&mut base, 1);
|
||||
// Point "hello.txt" at inode 24: group 1, slot 7, which starts 128 bytes before the end.
|
||||
let at = base.windows(9).position(|w| w == b"hello.txt").expect("the fixture lists hello.txt");
|
||||
base[at - 8..at - 4].copy_from_slice(&24u32.to_le_bytes());
|
||||
let img = read(&base, &Limits::default());
|
||||
assert!(img.entries.iter().all(|e| e.path != "hello.txt"), "{:?}", img.entries);
|
||||
assert!(img.entries.iter().any(|e| e.path == "sub"), "the rest of the directory is still read");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hostile_never_panics() {
|
||||
let base = craft_anything_testkit::firmware::ext_image(false);
|
||||
|
||||
@@ -146,6 +146,10 @@ Owner: open · Status: done (branch chore/r0.5-cleanups)
|
||||
- [x] Missing `database` icon for SQLite in the UI (`kind_icon`)
|
||||
- [x] Deduplicate the five private `fn spec(` helpers in the tool modules
|
||||
- [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)
|
||||
|
||||
### R0.8 Tests never touch the real machine
|
||||
Owner: open · Status: done, except protecting `main` (a maintainer setting)
|
||||
|
||||
Reference in New Issue
Block a user