diff --git a/crates/clawhdf5-remote/src/cache.rs b/crates/clawhdf5-remote/src/cache.rs index a434814..151cc84 100644 --- a/crates/clawhdf5-remote/src/cache.rs +++ b/crates/clawhdf5-remote/src/cache.rs @@ -305,9 +305,9 @@ impl BlockCache { let end = offset.saturating_add(bytes.len() as u64).min(self.len); let mut i = offset.div_ceil(bs); let mut st = lock(&self.state); - while i * bs < end { - let start = i * bs; - let block_end = (start + bs).min(self.len); + // Checked: a hostile server can claim a length near u64::MAX. + while let Some(start) = i.checked_mul(bs).filter(|&s| s < end) { + let block_end = start.saturating_add(bs).min(self.len); if block_end > end { break; } @@ -330,9 +330,14 @@ impl BlockCache { Some(offset / bs..(end - 1) / bs + 1) } + /// Length of block `i` (0 past the end of the file). Saturating: the + /// length may be anything a server claimed, up to `u64::MAX`. fn block_len(&self, i: u64) -> u64 { - let start = i * self.config.block_size; - (start + self.config.block_size).min(self.len) - start + let start = i.saturating_mul(self.config.block_size); + start + .saturating_add(self.config.block_size) + .min(self.len) + .saturating_sub(start) } fn keep(&self, st: &mut State, i: u64, data: Block) { @@ -453,8 +458,17 @@ impl BlockCache { let mut i = run.start / bs; let mut pos = 0usize; while pos < bytes.len() { - let n = self.block_len(i) as usize; - got.push((i, Arc::from(&bytes[pos..pos + n]))); + let n = usize::try_from(self.block_len(i)).unwrap_or(usize::MAX); + let Some(block) = pos.checked_add(n).and_then(|e| bytes.get(pos..e)) else { + return Err(guard.fail(FormatError::Storage(format!( + "a run at offset {} does not split into whole blocks", + run.start + )))); + }; + if n == 0 { + break; + } + got.push((i, Arc::from(block))); pos += n; i += 1; } @@ -512,7 +526,9 @@ impl BlockCache { } } runs.into_iter() - .map(|(a, b)| a * bs..(b * bs + self.block_len(b))) + .map(|(a, b)| { + a.saturating_mul(bs)..b.saturating_mul(bs).saturating_add(self.block_len(b)) + }) .collect() } @@ -863,4 +879,42 @@ mod tests { assert_eq!(n, blocks.len(), "a block was fetched twice: {fetched:?}"); assert!(c.stats().waits > 0, "readers should have shared fetches"); } + + /// A backend claiming any length (a hostile server's `Content-Range`) + /// and serving zeros. + struct Zeros(u64); + + impl Storage for Zeros { + fn read_at(&self, offset: u64, len: usize) -> Result, FormatError> { + let end = offset.saturating_add(len as u64).min(self.0); + Ok(Cow::Owned(vec![0; end.saturating_sub(offset) as usize])) + } + fn len(&self) -> u64 { + self.0 + } + } + + #[test] + fn lengths_near_u64_max_do_not_overflow() { + for len in [u64::MAX, u64::MAX - 1, u64::MAX - 1000, 1 << 63] { + let c = BlockCache::new(Zeros(len), small()); + for (off, n) in [ + (len - 100, 50), + (len - 10, 100), + (len - 1, 1), + (len - 3000, 3000), + (u64::MAX, 10), + ] { + let got = c.read_at(off, n).unwrap(); + assert_eq!(got.len() as u64, len.saturating_sub(off).min(n as u64)); + } + let got = c + .read_ranges(&[len - 5000..len, u64::MAX - 1..u64::MAX]) + .unwrap(); + assert_eq!(got[0].len(), 5000); + c.insert(len - 5, &[0; 5]); + c.insert(u64::MAX - 5, &[0; 5]); + c.prefetch(len - 1, 10).unwrap(); + } + } } diff --git a/crates/clawhdf5-remote/tests/common/server.rs b/crates/clawhdf5-remote/tests/common/server.rs index a060a9d..13f279a 100644 --- a/crates/clawhdf5-remote/tests/common/server.rs +++ b/crates/clawhdf5-remote/tests/common/server.rs @@ -48,6 +48,10 @@ pub struct Shared { pub fail_next: AtomicU32, /// Sleep this long before answering each request. pub delay_ms: AtomicU64, + /// When non-zero, claim the file is this long (in `Content-Range` and + /// when checking ranges) and serve zeros past its real end: a hostile + /// server lying about the length. + pub fake_total: AtomicU64, /// Requests for a served path (every status); requests for other /// paths are not counted. pub requests: AtomicU64, @@ -262,7 +266,8 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> { )?; continue; } - let len = data.len() as u64; + let fake = s.fake_total.load(Ordering::SeqCst); + let len = if fake > 0 { fake } else { data.len() as u64 }; let mut validators = String::new(); if s.weak_etag.load(Ordering::SeqCst) { validators = format!("ETag: W/{etag}\r\n"); @@ -291,6 +296,7 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> { .unwrap() .push((path.clone(), range.and_then(Result::ok))); } + let mut padded = Vec::new(); let (status, body, extra) = match range { Some(Err(())) => { write!( @@ -302,7 +308,7 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> { } Some(Ok((a, b))) => ( "206 Partial Content", - &data[a as usize..=b as usize], + slice_or_zeros(&data, a, b, &mut padded), format!("Content-Range: bytes {a}-{b}/{len}\r\n"), ), None => ("200 OK", &data[..], String::new()), @@ -342,3 +348,18 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> { } } } + +/// `data[a..=b]`, padded with zeros past its end (into `padded`). +fn slice_or_zeros<'a>(data: &'a [u8], a: u64, b: u64, padded: &'a mut Vec) -> &'a [u8] { + let real = data.len() as u64; + if b < real { + return &data[a as usize..=b as usize]; + } + let n = usize::try_from(b - a + 1).expect("range fits in memory"); + padded.resize(n, 0); + if a < real { + let have = (real - a) as usize; + padded[..have].copy_from_slice(&data[a as usize..]); + } + padded +} diff --git a/crates/clawhdf5-remote/tests/http.rs b/crates/clawhdf5-remote/tests/http.rs index 2c95cd1..2955f54 100644 --- a/crates/clawhdf5-remote/tests/http.rs +++ b/crates/clawhdf5-remote/tests/http.rs @@ -544,3 +544,36 @@ fn requests_for_other_paths_are_not_counted() { assert_eq!(server.requests(), 1, "only the open counts"); assert_eq!(server.log().len(), 1); } + +/// A server claiming a length near `u64::MAX` (and serving zeros past the +/// real data), with a file whose addresses point at the end of that range: +/// clean errors or zeros, never an arithmetic overflow (a panic in debug). +#[test] +fn a_server_claiming_a_huge_length_does_not_overflow() { + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); + let mut bytes = + std::fs::read(root.join("../clawhdf5-format/tests/fixtures/legacy/h5ex_g_iterate.h5")) + .unwrap(); + // Superblock v0: end-of-file address, then the root symbol table + // entry's object header, B-tree and heap addresses. + bytes[0x28..0x30].copy_from_slice(&(u64::MAX - 1).to_le_bytes()); + for at in [0x40, 0x50, 0x58] { + bytes[at..at + 8].copy_from_slice(&0xFFFF_FFFF_FFFF_F000u64.to_le_bytes()); + } + let server = Server::start(vec![("/h.h5".into(), bytes)]); + for total in [u64::MAX, u64::MAX - 1, 1 << 62] { + server.shared.fake_total.store(total, Ordering::SeqCst); + let url = server.url("/h.h5"); + let storage = storage_for_url(&url, &quick()).unwrap(); + assert_eq!(clawhdf5_format::storage::Storage::len(&*storage), total); + for (off, n) in [(total - 100, 50), (total - 10, 100), (total - 1, 1)] { + let got = clawhdf5_format::storage::Storage::read_at(&*storage, off, n).unwrap(); + assert_eq!(got.len() as u64, (total - off).min(n as u64)); + assert!(got.iter().all(|&b| b == 0)); + } + if let Ok(f) = File::open_storage(storage) { + let _ = transcript(&f); + } + let _ = open_url_with(&url, &quick()).map(|f| transcript(&f)); + } +}