clawhdf5-remote: no overflow on lengths near u64::MAX
A server can claim any length in Content-Range. block_len computed start + block_size, which overflowed in the last blocks of a file claimed to be near u64::MAX (a panic in debug builds, a wrapped value in release); insert() multiplied block indices unchecked. The cache's block arithmetic is now saturating/checked, and a run that does not split into whole blocks is an error instead of an endless loop or a slice panic. The test server gains fake_total (claim a length, serve zeros past the data); a test reads the last bytes of such files and opens a file whose superblock EOF and root addresses sit near u64::MAX. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -305,9 +305,9 @@ impl<S: Storage> BlockCache<S> {
|
|||||||
let end = offset.saturating_add(bytes.len() as u64).min(self.len);
|
let end = offset.saturating_add(bytes.len() as u64).min(self.len);
|
||||||
let mut i = offset.div_ceil(bs);
|
let mut i = offset.div_ceil(bs);
|
||||||
let mut st = lock(&self.state);
|
let mut st = lock(&self.state);
|
||||||
while i * bs < end {
|
// Checked: a hostile server can claim a length near u64::MAX.
|
||||||
let start = i * bs;
|
while let Some(start) = i.checked_mul(bs).filter(|&s| s < end) {
|
||||||
let block_end = (start + bs).min(self.len);
|
let block_end = start.saturating_add(bs).min(self.len);
|
||||||
if block_end > end {
|
if block_end > end {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
@@ -330,9 +330,14 @@ impl<S: Storage> BlockCache<S> {
|
|||||||
Some(offset / bs..(end - 1) / bs + 1)
|
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 {
|
fn block_len(&self, i: u64) -> u64 {
|
||||||
let start = i * self.config.block_size;
|
let start = i.saturating_mul(self.config.block_size);
|
||||||
(start + self.config.block_size).min(self.len) - start
|
start
|
||||||
|
.saturating_add(self.config.block_size)
|
||||||
|
.min(self.len)
|
||||||
|
.saturating_sub(start)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn keep(&self, st: &mut State, i: u64, data: Block) {
|
fn keep(&self, st: &mut State, i: u64, data: Block) {
|
||||||
@@ -453,8 +458,17 @@ impl<S: Storage> BlockCache<S> {
|
|||||||
let mut i = run.start / bs;
|
let mut i = run.start / bs;
|
||||||
let mut pos = 0usize;
|
let mut pos = 0usize;
|
||||||
while pos < bytes.len() {
|
while pos < bytes.len() {
|
||||||
let n = self.block_len(i) as usize;
|
let n = usize::try_from(self.block_len(i)).unwrap_or(usize::MAX);
|
||||||
got.push((i, Arc::from(&bytes[pos..pos + n])));
|
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;
|
pos += n;
|
||||||
i += 1;
|
i += 1;
|
||||||
}
|
}
|
||||||
@@ -512,7 +526,9 @@ impl<S: Storage> BlockCache<S> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
runs.into_iter()
|
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()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -863,4 +879,42 @@ mod tests {
|
|||||||
assert_eq!(n, blocks.len(), "a block was fetched twice: {fetched:?}");
|
assert_eq!(n, blocks.len(), "a block was fetched twice: {fetched:?}");
|
||||||
assert!(c.stats().waits > 0, "readers should have shared fetches");
|
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<Cow<'_, [u8]>, 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();
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -48,6 +48,10 @@ pub struct Shared {
|
|||||||
pub fail_next: AtomicU32,
|
pub fail_next: AtomicU32,
|
||||||
/// Sleep this long before answering each request.
|
/// Sleep this long before answering each request.
|
||||||
pub delay_ms: AtomicU64,
|
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
|
/// Requests for a served path (every status); requests for other
|
||||||
/// paths are not counted.
|
/// paths are not counted.
|
||||||
pub requests: AtomicU64,
|
pub requests: AtomicU64,
|
||||||
@@ -262,7 +266,8 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> {
|
|||||||
)?;
|
)?;
|
||||||
continue;
|
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();
|
let mut validators = String::new();
|
||||||
if s.weak_etag.load(Ordering::SeqCst) {
|
if s.weak_etag.load(Ordering::SeqCst) {
|
||||||
validators = format!("ETag: W/{etag}\r\n");
|
validators = format!("ETag: W/{etag}\r\n");
|
||||||
@@ -291,6 +296,7 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> {
|
|||||||
.unwrap()
|
.unwrap()
|
||||||
.push((path.clone(), range.and_then(Result::ok)));
|
.push((path.clone(), range.and_then(Result::ok)));
|
||||||
}
|
}
|
||||||
|
let mut padded = Vec::new();
|
||||||
let (status, body, extra) = match range {
|
let (status, body, extra) = match range {
|
||||||
Some(Err(())) => {
|
Some(Err(())) => {
|
||||||
write!(
|
write!(
|
||||||
@@ -302,7 +308,7 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> {
|
|||||||
}
|
}
|
||||||
Some(Ok((a, b))) => (
|
Some(Ok((a, b))) => (
|
||||||
"206 Partial Content",
|
"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"),
|
format!("Content-Range: bytes {a}-{b}/{len}\r\n"),
|
||||||
),
|
),
|
||||||
None => ("200 OK", &data[..], String::new()),
|
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<u8>) -> &'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
|
||||||
|
}
|
||||||
|
|||||||
@@ -544,3 +544,36 @@ fn requests_for_other_paths_are_not_counted() {
|
|||||||
assert_eq!(server.requests(), 1, "only the open counts");
|
assert_eq!(server.requests(), 1, "only the open counts");
|
||||||
assert_eq!(server.log().len(), 1);
|
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));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user