From c8b35543794fe176d34626233ba90d566b7ff05c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 01:40:52 +0000 Subject: [PATCH] fix(if-image): bound CHD hunk count before allocating the map Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016hSe4aXRfBbzBVFFJfsNAY --- AUDIT.md | 10 ++++++++++ CHANGELOG.md | 4 ++++ crates/if-image/src/chd.rs | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/AUDIT.md b/AUDIT.md index d0d0df8..bd92e82 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -43,6 +43,16 @@ Each feature is listed as Verified, Partially verified or Unverified, with the e | B-8 | Dependency audit (`cargo audit` over the lockfile): could not be run for this pass — the audit tool would not install on the development VM (network transfer failures, twice; a third attempt at the 1.0 pass stalled the same way). The lockfile's crates are the workspace's long-standing set; no new dependency was added by M6 or RC. | — | Owed: run `cargo audit` in the repo and record the result here. | | B-9 | Image parsers take malformed input without panicking (RC drills: empty, truncated, garbage and random files through open/walk). A dedicated fuzz campaign (cargo-fuzz over the ISO/UDF/CHD/CCD parsers) has not been run. | Medium | Drills in place; fuzz campaign still owed. | +## 2026-11-09 — Panic-path review + +**Scope:** every non-test `unwrap`/`expect`/slice-index/allocation in the workspace, checked for reachability from untrusted input (image files, disk contents). `panic = "abort"` makes any panic fatal. + +| # | Finding | Severity | Status | +|---|---|---|---| +| B-11 | A CHD header's logical size sized the hunk-map allocation unchecked: a hostile header (logical size 2^60) aborted the process while opening the file. | Medium | Fixed: hunk count capped, uncompressed map checked against the file length; regression test fails without the fix. | +| B-12 | `Fat32Volume::open` computes `total - data_start` and sizes the FAT from `fat_sectors` without checking them against the volume. It only opens volumes this program just wrote, so no untrusted path reaches it today. | Low | Open: harden before any "read an existing stick" feature. | +| B-13 | The remaining `unwrap`/`expect` calls are on fixed-size slices, static algorithm names, locks and invariants set a line earlier. None found reachable from file input. | — | Reviewed by reading, not by fuzzing; B-9 stays open. | + ## 2026-11-07 — M0 shell | # | Finding | Severity | Status | diff --git a/CHANGELOG.md b/CHANGELOG.md index 2153eef..7cc48cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes are listed here. Versions follow [Semantic Versioning](https ## [Unreleased] +### Fixed + +- Opening a CHD whose header declared an absurd size could abort the program while allocating the hunk map. The header is now checked first (AUDIT B-11). + ### Docs - Added Ventoy, YUMI, Universal USB Installer, XBOOT, LiveUSB Install and balenaEtcher to the references, roadmap (multiboot and USB-safety items) and credits. Ideas only; no code used. diff --git a/crates/if-image/src/chd.rs b/crates/if-image/src/chd.rs index fa39694..a981831 100644 --- a/crates/if-image/src/chd.rs +++ b/crates/if-image/src/chd.rs @@ -804,6 +804,9 @@ pub struct Chd { disc: Option, } +/// Upper bound on hunks in one CHD (64 Mi hunks: far beyond any real disc). +const MAX_HUNKS: usize = 1 << 26; + impl Chd { pub fn open(path: &Path) -> Result { let mut f = File::open(path)?; @@ -831,8 +834,16 @@ impl Chd { } let hunks = logical.div_ceil(hunkbytes as u64) as usize; let len = f.metadata()?.len(); + // The hunk count comes straight from the header; bound it before it + // sizes any allocation, so a hostile header cannot exhaust memory. + if hunks > MAX_HUNKS { + return bad("CHD header declares an implausible number of hunks"); + } let map = if codecs[0] == 0 { // Uncompressed CHD: 4-byte big-endian hunk offsets (in hunks). + if map_off.checked_add(hunks as u64 * 4).is_none_or(|end| end > len) { + return bad("CHD map outside the file"); + } let mut m = vec![0u8; hunks * 4]; f.seek(SeekFrom::Start(map_off))?; f.read_exact(&mut m)?; @@ -1108,4 +1119,26 @@ mod tests { assert_eq!(c[0], 0b1101_1111); assert_eq!(cdzl_decompress(&c, hunk.len()).unwrap(), hunk); } + + /// A header that claims an absurd logical size must be refused, not + /// turned into a giant allocation (and, with panic=abort, a crash). + #[test] + fn hostile_header_is_refused_before_allocating() { + for codec0 in [0u32, u32::from_be_bytes(*b"zlib")] { + let mut h = [0u8; 124]; + h[0..8].copy_from_slice(b"MComprHD"); + h[8..12].copy_from_slice(&124u32.to_be_bytes()); + h[12..16].copy_from_slice(&5u32.to_be_bytes()); + h[16..20].copy_from_slice(&codec0.to_be_bytes()); + h[32..40].copy_from_slice(&(1u64 << 60).to_be_bytes()); // logical bytes + h[40..48].copy_from_slice(&124u64.to_be_bytes()); // map offset + h[56..60].copy_from_slice(&512u32.to_be_bytes()); // hunk bytes + h[60..64].copy_from_slice(&512u32.to_be_bytes()); // unit bytes + let dir = std::env::temp_dir().join(format!("if-chd-hostile-{}-{codec0}", std::process::id())); + std::fs::write(&dir, h).unwrap(); + let r = Chd::open(&dir); + std::fs::remove_file(&dir).ok(); + assert!(matches!(r, Err(Error::Format(_))), "codec {codec0}: {:?}", r.err()); + } + } }