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 <noreply@anthropic.com>
This commit is contained in:
parent
c1c3e7d6fe
commit
32bc2367da
2 changed files with 21 additions and 3 deletions
|
|
@ -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<Vec<u8>> {
|
||||
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") {
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue