From 32bc2367da668baaa84afd9849b1602a398ee7a5 Mon Sep 17 00:00:00 2001 From: Eureka Date: Sun, 8 Nov 2026 08:13:35 +0000 Subject: [PATCH] chat_image: the bounded read is its own function, with a test that fails without the take bound load() refuses an over-cap file on its length first, so the take(MAX_BYTES + 1) on the read was reachable only by a file that grew after that check and no test could see it. read_bounded holds the read; its test grows a file past the cap and requires the read to stop at MAX_BYTES + 1. Co-Authored-By: Claude Sonnet 5.5 --- crates/mind-body/src/chat_image.rs | 12 +++++++++--- crates/mind-body/src/chat_image/tests.rs | 12 ++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/crates/mind-body/src/chat_image.rs b/crates/mind-body/src/chat_image.rs index 85fd217..da26e2a 100644 --- a/crates/mind-body/src/chat_image.rs +++ b/crates/mind-body/src/chat_image.rs @@ -97,9 +97,7 @@ pub(crate) fn load(store: Option<&Path>, room_id: &str, image: &ChatImage) -> Re if meta.len() > MAX_BYTES { return Err(Skip::TooLarge); } - let mut bytes = Vec::new(); - // A file that grew since the check reads as another size than the declared one. - File::open(&wanted).map_err(|_| Skip::Missing)?.take(MAX_BYTES + 1).read_to_end(&mut bytes).map_err(|_| Skip::Missing)?; + let bytes = read_bounded(&wanted).map_err(|_| Skip::Missing)?; if bytes.len() as u64 != image.size { return Err(Skip::SizeMismatch); } @@ -113,6 +111,14 @@ pub(crate) fn load(store: Option<&Path>, room_id: &str, image: &ChatImage) -> Re Ok(Image { media_type, base64: STANDARD.encode(&bytes) }) } +/// At most `MAX_BYTES + 1` bytes of `path`: a file that grew since the length check reads as another size than the +/// declared one, and never as an unbounded allocation. +fn read_bounded(path: &Path) -> std::io::Result> { + let mut bytes = Vec::new(); + File::open(path)?.take(MAX_BYTES + 1).read_to_end(&mut bytes)?; + Ok(bytes) +} + /// The media type the first bytes say, among the four a model takes. fn sniff(bytes: &[u8]) -> Option<&'static str> { if bytes.starts_with(b"\x89PNG\r\n\x1a\n") { diff --git a/crates/mind-body/src/chat_image/tests.rs b/crates/mind-body/src/chat_image/tests.rs index 17f1fe8..9966f80 100644 --- a/crates/mind-body/src/chat_image/tests.rs +++ b/crates/mind-body/src/chat_image/tests.rs @@ -221,3 +221,15 @@ fn a_fifo_where_the_file_should_be_is_refused_without_opening_it() { std::thread::spawn(move || tx.send(load(Some(&root), ROOM, &image)).ok()); assert_eq!(rx.recv_timeout(std::time::Duration::from_secs(5)).expect("load returned without waiting on the FIFO"), Err(Skip::Missing)); } + +#[test] +fn a_file_that_outgrew_its_length_check_is_read_no_further_than_one_byte_past_the_cap() { + // `load` refuses an oversize file on its length first, so only a file that grows after that check reaches the + // read; the read itself must stop at MAX_BYTES + 1 (enough to disagree with any declared size, never unbounded). + let store = Store::new(); + let path = store.root().join("grew"); + std::fs::write(&path, vec![0u8; MAX_BYTES as usize + 4096]).unwrap(); + assert_eq!(read_bounded(&path).unwrap().len() as u64, MAX_BYTES + 1); + std::fs::write(&path, PNG).unwrap(); + assert_eq!(read_bounded(&path).unwrap(), PNG, "a file within the cap is read whole"); +}