fix(format): overflow-checked sizes and fallible allocation on chunked reads

Dataspace and chunk dimensions are untrusted 64-bit fields, but the chunked
read paths computed `num_elements() as usize * elem_size` and
`chunk_dims.product() * elem_size` with plain arithmetic and fed the result to
`vec![0u8; n]`. A crafted file could wrap the product (under-sizing the output
buffer that chunks are then copied into) or request an allocation large enough
to abort the process.

- Dataspace::checked_num_elements, checked_byte_len, checked_chunk_byte_len
  and alloc_output (try_reserve_exact) replace the plain products and
  vec![0; n] at every chunked read site, plus the VDS and hyperslab paths.
  Overflow and allocation failure are FormatError::Overflow.
- Dataspace::num_elements saturates instead of wrapping.
- A zero-element dataset returns early, which also keeps the stride products
  in range when another dimension is huge.
- parallel_read.rs: the three `c_addr + size > len` bounds checks used a raw
  add; they now use checked_add like the rest of the crate.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

crates/clawhdf5-format/src/chunked_read.rs

@@ -132,6 +132,47 @@ fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatErr
     Ok(())
 }
 
+/// `elements * elem_size` for sizes that come from the file. Dataspace and
+/// chunk dimensions are untrusted 64-bit fields, so a crafted file can make
+/// the plain product wrap to a small number (or to something enormous).
+pub(crate) fn checked_byte_len(elements: u64, elem_size: usize) -> Result<usize, FormatError> {
+    usize::try_from(elements)
+        .ok()
+        .and_then(|n| n.checked_mul(elem_size))
+        .ok_or_else(|| {
+            FormatError::Overflow(format!(
+                "{elements} elements of {elem_size} bytes exceeds the addressable size"
+            ))
+        })
+}
+
+/// Product of chunk dimensions times the element size, overflow-checked.
+pub(crate) fn checked_chunk_byte_len(
+    chunk_dims: &[usize],
+    elem_size: usize,
+) -> Result<usize, FormatError> {
+    chunk_dims
+        .iter()
+        .try_fold(elem_size, |acc, &d| acc.checked_mul(d))
+        .ok_or_else(|| {
+            FormatError::Overflow(format!(
+                "chunk dimensions {chunk_dims:?} x {elem_size} bytes exceeds the addressable size"
+            ))
+        })
+}
+
+/// A zero-filled output buffer of `len` bytes. `vec![0; len]` aborts the
+/// process when the allocation fails; a size taken from the file must surface
+/// as an error instead.
+pub(crate) fn alloc_output(len: usize) -> Result<Vec<u8>, FormatError> {
+    let mut out = Vec::new();
+    out.try_reserve_exact(len).map_err(|_| {
+        FormatError::Overflow(format!("cannot allocate {len} bytes for dataset output"))
+    })?;
+    out.resize(len, 0);
+    Ok(out)
+}
+
 fn read_offset(data: &[u8], pos: usize, size: u8) -> Result<u64, FormatError> {
     let s = size as usize;
     if pos.checked_add(s).is_none_or(|end| end > data.len()) {
@@ -393,7 +434,7 @@ pub fn read_chunked_data(
         }
         (4, Some(1)) => {
             // Single chunk — one chunk covering the entire dataset
-            let chunk_byte_size: usize = chunk_dims.iter().product::<usize>() * elem_size;
+            let chunk_byte_size = checked_chunk_byte_len(&chunk_dims, elem_size)?;
             let (csize, fmask) = if let Some(fs) = single_filtered_size {
                 (fs as u32, single_filter_mask.unwrap_or(0))
             } else {
@@ -454,9 +495,13 @@ pub fn read_chunked_data(
     };
 
     // Assemble output
-    let total_elements = dataspace.num_elements() as usize;
+    let total_bytes = checked_byte_len(dataspace.checked_num_elements()?, elem_size)?;
-    let total_bytes = total_elements * elem_size;
+    if total_bytes == 0 {
-    let mut output = vec![0u8; total_bytes];
+        // Also keeps the stride products below in range: with a zero-sized
+        // dimension the total is 0 even if other dimensions are huge.
+        return Ok(Vec::new());
+    }
+    let mut output = alloc_output(total_bytes)?;
 
     let mut ds_strides = vec![1usize; rank];
     for i in (0..rank.saturating_sub(1)).rev() {
@@ -468,8 +513,7 @@ pub fn read_chunked_data(
         chunk_strides[i] = chunk_strides[i + 1] * chunk_dims[i + 1];
     }
 
-    let chunk_total_elements: usize = chunk_dims.iter().product();
+    let chunk_total_bytes = checked_chunk_byte_len(&chunk_dims, elem_size)?;
-    let chunk_total_bytes = chunk_total_elements * elem_size;
 
     // Fast path: no filters — copy directly from file_data without intermediate alloc
     if pipeline.is_none() {
@@ -623,7 +667,7 @@ pub fn read_chunked_data_cached(
         let chunks = match (version, chunk_index_type) {
             (3, _) => collect_chunk_info(file_data, addr, ndims, offset_size, length_size)?,
             (4, Some(1)) => {
-                let chunk_byte_size: usize = chunk_dims.iter().product::<usize>() * elem_size;
+                let chunk_byte_size = checked_chunk_byte_len(&chunk_dims, elem_size)?;
                 let (csize, fmask) = if let Some(fs) = single_filtered_size {
                     (fs as u32, single_filter_mask.unwrap_or(0))
                 } else {
@@ -689,9 +733,13 @@ pub fn read_chunked_data_cached(
     let chunks = cache.all_indexed_chunks().unwrap_or_default();
 
     // Assemble output
-    let total_elements = dataspace.num_elements() as usize;
+    let total_bytes = checked_byte_len(dataspace.checked_num_elements()?, elem_size)?;
-    let total_bytes = total_elements * elem_size;
+    if total_bytes == 0 {
-    let mut output = vec![0u8; total_bytes];
+        // Also keeps the stride products below in range: with a zero-sized
+        // dimension the total is 0 even if other dimensions are huge.
+        return Ok(Vec::new());
+    }
+    let mut output = alloc_output(total_bytes)?;
 
     let mut ds_strides = vec![1usize; rank];
     for i in (0..rank.saturating_sub(1)).rev() {
@@ -703,8 +751,7 @@ pub fn read_chunked_data_cached(
         chunk_strides[i] = chunk_strides[i + 1] * chunk_dims[i + 1];
     }
 
-    let chunk_total_elements: usize = chunk_dims.iter().product();
+    let chunk_total_bytes = checked_chunk_byte_len(&chunk_dims, elem_size)?;
-    let chunk_total_bytes = chunk_total_elements * elem_size;
 
     for chunk_info in &chunks {
         let coord: Vec<u64> = chunk_info.offsets.iter().take(rank).copied().collect();
@@ -976,7 +1023,7 @@ pub fn read_chunked_data_sweep(
         let chunks = match (version, chunk_index_type) {
             (3, _) => collect_chunk_info(file_data, addr, ndims, offset_size, length_size)?,
             (4, Some(1)) => {
-                let chunk_byte_size: usize = chunk_dims.iter().product::<usize>() * elem_size;
+                let chunk_byte_size = checked_chunk_byte_len(&chunk_dims, elem_size)?;
                 let (csize, fmask) = if let Some(fs) = single_filtered_size {
                     (fs as u32, single_filter_mask.unwrap_or(0))
                 } else {
@@ -1042,9 +1089,13 @@ pub fn read_chunked_data_sweep(
     let chunks = cache.all_indexed_chunks().unwrap_or_default();
 
     // Assemble output
-    let total_elements = dataspace.num_elements() as usize;
+    let total_bytes = checked_byte_len(dataspace.checked_num_elements()?, elem_size)?;
-    let total_bytes = total_elements * elem_size;
+    if total_bytes == 0 {
-    let mut output = vec![0u8; total_bytes];
+        // Also keeps the stride products below in range: with a zero-sized
+        // dimension the total is 0 even if other dimensions are huge.
+        return Ok(Vec::new());
+    }
+    let mut output = alloc_output(total_bytes)?;
 
     let mut ds_strides = vec![1usize; rank];
     for i in (0..rank.saturating_sub(1)).rev() {
@@ -1056,8 +1107,7 @@ pub fn read_chunked_data_sweep(
         chunk_strides[i] = chunk_strides[i + 1] * chunk_dims[i + 1];
     }
 
-    let chunk_total_elements: usize = chunk_dims.iter().product();
+    let chunk_total_bytes = checked_chunk_byte_len(&chunk_dims, elem_size)?;
-    let chunk_total_bytes = chunk_total_elements * elem_size;
 
     for chunk_info in &chunks {
         let coord: Vec<u64> = chunk_info.offsets.iter().take(rank).copied().collect();
@@ -1199,7 +1249,7 @@ pub fn read_chunked_data_indexed(
         let chunks = match (version, chunk_index_type) {
             (3, _) => collect_chunk_info(file_data, addr, ndims, offset_size, length_size)?,
             (4, Some(1)) => {
-                let chunk_byte_size: usize = chunk_dims.iter().product::<usize>() * elem_size;
+                let chunk_byte_size = checked_chunk_byte_len(&chunk_dims, elem_size)?;
                 let (csize, fmask) = if let Some(fs) = single_filtered_size {
                     (fs as u32, single_filter_mask.unwrap_or(0))
                 } else {
@@ -1463,6 +1513,64 @@ fn copy_chunk_to_output(
 mod tests {
     use super::*;
 
+    fn simple_space(dimensions: Vec<u64>) -> Dataspace {
+        Dataspace {
+            space_type: crate::dataspace::DataspaceType::Simple,
+            rank: dimensions.len() as u8,
+            dimensions,
+            max_dimensions: None,
+        }
+    }
+
+    #[test]
+    fn crafted_dimensions_are_errors_not_wraparound() {
+        // 2^63 * 2 wraps to 0 with a plain product; 2^40 * 2^40 wraps too.
+        for dims in [
+            vec![1u64 << 63, 2],
+            vec![1 << 40, 1 << 40],
+            vec![u64::MAX, u64::MAX],
+        ] {
+            let space = simple_space(dims.clone());
+            assert!(
+                matches!(space.checked_num_elements(), Err(FormatError::Overflow(_))),
+                "{dims:?}"
+            );
+            // The infallible accessor saturates instead of wrapping.
+            assert_eq!(space.num_elements(), u64::MAX, "{dims:?}");
+        }
+        assert_eq!(simple_space(vec![3, 4]).checked_num_elements().unwrap(), 12);
+        // A zero-sized dimension makes the whole product 0, not an overflow.
+        assert_eq!(
+            simple_space(vec![0, 1 << 40, 1 << 40])
+                .checked_num_elements()
+                .unwrap(),
+            0
+        );
+    }
+
+    #[test]
+    fn byte_length_helpers_check_overflow() {
+        assert_eq!(checked_byte_len(10, 8).unwrap(), 80);
+        assert!(matches!(
+            checked_byte_len(u64::MAX, 8),
+            Err(FormatError::Overflow(_))
+        ));
+        assert_eq!(checked_chunk_byte_len(&[10, 10], 4).unwrap(), 400);
+        assert!(matches!(
+            checked_chunk_byte_len(&[usize::MAX, 2], 4),
+            Err(FormatError::Overflow(_))
+        ));
+    }
+
+    #[test]
+    fn unallocatable_output_is_an_error_not_an_abort() {
+        assert_eq!(alloc_output(16).unwrap(), vec![0u8; 16]);
+        assert!(matches!(
+            alloc_output(usize::MAX / 2),
+            Err(FormatError::Overflow(_))
+        ));
+    }
+
     fn write_offset(buf: &mut Vec<u8>, val: u64, size: u8) {
         match size {
             4 => buf.extend_from_slice(&(val as u32).to_le_bytes()),

crates/clawhdf5-format/src/data_read.rs

@@ -475,8 +475,10 @@ fn read_virtual_data(
     use crate::selection::Selection;
 
     let elem_size = datatype.type_size() as usize;
-    let total_elems = dataspace.num_elements() as usize;
+    let mut out = crate::chunked_read::alloc_output(crate::chunked_read::checked_byte_len(
-    let mut out = vec![0u8; total_elems.saturating_mul(elem_size)];
+        dataspace.checked_num_elements()?,
+        elem_size,
+    )?)?;
 
     let virtual_dims = &dataspace.dimensions;
 
@@ -616,12 +618,14 @@ fn extract_selection_from_buffer(
             block,
         } => {
             let rank = dims.len();
-            let output_elements: usize = count
+            let output_elements = count
                 .iter()
                 .zip(block.iter())
-                .map(|(&c, &b)| (c * b) as usize)
+                .try_fold(1u64, |acc, (&c, &b)| acc.checked_mul(c.checked_mul(b)?))
-                .product();
+                .ok_or_else(|| FormatError::Overflow("hyperslab count x block overflows".into()))?;
-            let mut output = vec![0u8; output_elements * elem_size];
+            let mut output = crate::chunked_read::alloc_output(
+                crate::chunked_read::checked_byte_len(output_elements, elem_size)?,
+            )?;
 
             // Compute dataset strides (row-major)
             let mut ds_strides = vec![1usize; rank];

crates/clawhdf5-format/src/dataspace.rs

@@ -1,5 +1,7 @@
 //! HDF5 Dataspace message parsing (message type 0x0001).
 
+#[cfg(not(feature = "std"))]
+use alloc::format;
 #[cfg(not(feature = "std"))]
 use alloc::vec::Vec;
 
@@ -167,6 +169,27 @@ impl Dataspace {
         }
     }
 
+    /// [`Dataspace::num_elements`] with the product overflow-checked. The
+    /// dimensions are untrusted 64-bit fields; read paths that size a buffer
+    /// from them must use this one.
+    pub fn checked_num_elements(&self) -> Result<u64, FormatError> {
+        match self.space_type {
+            DataspaceType::Null => Ok(0),
+            DataspaceType::Scalar => Ok(1),
+            DataspaceType::Simple if self.dimensions.is_empty() => Ok(0),
+            DataspaceType::Simple => self
+                .dimensions
+                .iter()
+                .try_fold(1u64, |acc, &d| acc.checked_mul(d))
+                .ok_or_else(|| {
+                    FormatError::Overflow(format!(
+                        "dataspace dimensions {:?} overflow the element count",
+                        self.dimensions
+                    ))
+                }),
+        }
+    }
+
     /// Total number of elements. Scalar = 1, Null = 0.
     pub fn num_elements(&self) -> u64 {
         match self.space_type {
@@ -176,7 +199,12 @@ impl Dataspace {
                 if self.dimensions.is_empty() {
                     0
                 } else {
-                    self.dimensions.iter().product()
+                    // Saturate rather than wrap: a wrapped product could
+                    // under-size a buffer. Size-critical callers use
+                    // `checked_num_elements`.
+                    self.dimensions
+                        .iter()
+                        .fold(1u64, |acc, &d| acc.saturating_mul(d))
                 }
             }
         }

crates/clawhdf5-format/src/parallel_read.rs

@@ -73,9 +73,12 @@ pub fn decompress_chunks_lane_partitioned(
                 let c_addr = chunk_info.address as usize;
                 let size = chunk_info.chunk_size as usize;
 
-                if c_addr + size > file_data.len() {
+                if c_addr
+                    .checked_add(size)
+                    .is_none_or(|end| end > file_data.len())
+                {
                     return Err(FormatError::UnexpectedEof {
-                        expected: c_addr + size,
+                        expected: c_addr.saturating_add(size),
                         available: file_data.len(),
                     });
                 }
@@ -144,9 +147,12 @@ pub fn decompress_chunks_parallel(
         .map(|(index, chunk_info)| {
             let c_addr = chunk_info.address as usize;
             let size = chunk_info.chunk_size as usize;
-            if c_addr + size > file_data.len() {
+            if c_addr
+                .checked_add(size)
+                .is_none_or(|end| end > file_data.len())
+            {
                 return Err(FormatError::UnexpectedEof {
-                    expected: c_addr + size,
+                    expected: c_addr.saturating_add(size),
                     available: file_data.len(),
                 });
             }
@@ -182,9 +188,12 @@ pub fn decompress_chunks_sequential(
     for chunk_info in chunks {
         let c_addr = chunk_info.address as usize;
         let size = chunk_info.chunk_size as usize;
-        if c_addr + size > file_data.len() {
+        if c_addr
+            .checked_add(size)
+            .is_none_or(|end| end > file_data.len())
+        {
             return Err(FormatError::UnexpectedEof {
-                expected: c_addr + size,
+                expected: c_addr.saturating_add(size),
                 available: file_data.len(),
             });
         }