crates/clawhdf5-format/src/data_read.rs

@@ -32,6 +32,22 @@ fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatErr
     Ok(())
 }
 
+/// How many bytes to read from a contiguous dataset's storage of
+/// `storage_size` bytes (the layout message's size) holding `needed` bytes
+/// of elements. libhdf5 reads the elements' bytes from the start of the
+/// storage and ignores storage past them (`H5D__contig_check` checks only
+/// that the elements fit in the file), so a larger storage reads; one too
+/// small to hold the elements is an error.
+pub fn contiguous_read_len(storage_size: u64, needed: usize) -> Result<usize, FormatError> {
+    if storage_size < needed as u64 {
+        return Err(FormatError::DataSizeMismatch {
+            expected: needed,
+            actual: usize::try_from(storage_size).unwrap_or(usize::MAX),
+        });
+    }
+    Ok(needed)
+}
+
 /// Zero-copy read of contiguous raw data, returning a borrowed slice.
 ///
 /// For contiguous layouts, returns a direct `&[u8]` slice into `file_data`.
@@ -55,13 +71,7 @@ pub fn read_raw_data_zerocopy<'a>(
         DataLayout::Contiguous { address, size } => {
             let addr = address.ok_or(FormatError::NoDataAllocated)?;
             let addr = addr as usize;
-            let sz = *size as usize;
+            let sz = contiguous_read_len(*size, expected_size)?;
-            if sz != expected_size {
-                return Err(FormatError::DataSizeMismatch {
-                    expected: expected_size,
-                    actual: sz,
-                });
-            }
             ensure_len(file_data, addr, sz)?;
             Ok(Some(&file_data[addr..addr + sz]))
         }
@@ -172,13 +182,7 @@ fn read_raw_data_full_impl(
         DataLayout::Contiguous { address, size } => {
             let addr = address.ok_or(FormatError::NoDataAllocated)?;
             let addr = addr as usize;
-            let sz = *size as usize;
+            let sz = contiguous_read_len(*size, expected_size)?;
-            if sz != expected_size {
-                return Err(FormatError::DataSizeMismatch {
-                    expected: expected_size,
-                    actual: sz,
-                });
-            }
             ensure_len(file_data, addr, sz)?;
             let mut out = crate::bulk_alloc::vec_for_bulk(sz);
             out.extend_from_slice(&file_data[addr..addr + sz]);
@@ -2632,6 +2636,36 @@ mod tests {
         assert_eq!(result.unwrap(), &[1.5f32, 2.5, 3.5]);
     }
 
+    /// libhdf5 reads a contiguous dataset's elements from the start of its
+    /// storage and ignores storage past them (cve-2024-32623's scalar
+    /// `/Dset1` has 240 bytes of storage for one 4-byte element). Storage too
+    /// small for the elements is still an error.
+    #[test]
+    fn contiguous_storage_larger_than_the_elements_reads() {
+        let dt = make_f64_le_type();
+        let ds = make_simple_dataspace(&[2]);
+        let mut file_data = vec![0u8; 64];
+        file_data[..8].copy_from_slice(&1.5f64.to_le_bytes());
+        file_data[8..16].copy_from_slice(&2.5f64.to_le_bytes());
+        file_data[16..24].copy_from_slice(&9.0f64.to_le_bytes());
+        let layout = DataLayout::Contiguous {
+            address: Some(0),
+            size: 40,
+        };
+        let raw = read_raw_data(&file_data, &layout, &ds, &dt).unwrap();
+        assert_eq!(raw, file_data[..16]);
+        let zc = read_raw_data_zerocopy(&file_data, &layout, &ds, &dt).unwrap();
+        assert_eq!(zc, Some(&file_data[..16]));
+        let small = DataLayout::Contiguous {
+            address: Some(0),
+            size: 8,
+        };
+        assert!(matches!(
+            read_raw_data(&file_data, &small, &ds, &dt),
+            Err(FormatError::DataSizeMismatch { .. })
+        ));
+    }
+
     #[test]
     fn zerocopy_size_mismatch() {
         let dt = make_f64_le_type();

crates/clawhdf5-format/src/dataspace.rs

@@ -7,6 +7,9 @@ use alloc::vec::Vec;
 
 use crate::error::FormatError;
 
+/// Most dimensions a dataspace can have (`H5S_MAX_RANK`).
+pub const MAX_RANK: u8 = 32;
+
 /// Type of dataspace.
 #[derive(Debug, Clone, PartialEq)]
 pub enum DataspaceType {
@@ -67,6 +70,12 @@ impl Dataspace {
         let version = data[0];
         let rank = data[1];
         let flags = data[2];
+        // H5O__sdspace_decode's checks.
+        if rank > MAX_RANK {
+            return Err(FormatError::InvalidDataspace(
+                "simple dataspace dimensionality is too large",
+            ));
+        }
 
         let (space_type, header_size) = match version {
             1 => {
@@ -88,6 +97,11 @@ impl Dataspace {
                     2 => DataspaceType::Null,
                     _ => return Err(FormatError::InvalidDataspaceType(type_byte)),
                 };
+                if st != DataspaceType::Simple && rank > 0 {
+                    return Err(FormatError::InvalidDataspace(
+                        "invalid rank for scalar or NULL dataspace",
+                    ));
+                }
                 (st, 4usize)
             }
             _ => return Err(FormatError::InvalidDataspaceVersion(version)),
@@ -107,8 +121,13 @@ impl Dataspace {
         // Read max dimensions if flags bit 0 is set
         let max_dimensions = if flags & 0x01 != 0 {
             let mut max_dims = Vec::with_capacity(rank as usize);
-            for _ in 0..rank {
+            for i in 0..rank as usize {
                 let val = read_length(data, pos, length_size)?;
+                if dimensions[i] > val {
+                    return Err(FormatError::InvalidDataspace(
+                        "dataspace dimension size is greater than its maximum size",
+                    ));
+                }
                 max_dims.push(val);
                 pos += ls;
             }
@@ -176,7 +195,6 @@ impl Dataspace {
         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()
@@ -195,18 +213,14 @@ impl Dataspace {
         match self.space_type {
             DataspaceType::Null => 0,
             DataspaceType::Scalar => 1,
-            DataspaceType::Simple => {
+            // A simple dataspace of rank 0 holds one element, as in libhdf5
-                if self.dimensions.is_empty() {
+            // (the product of no dimensions). Saturate rather than wrap: a
-                    0
+            // wrapped product could under-size a buffer. Size-critical
-                } else {
+            // callers use `checked_num_elements`.
-                    // Saturate rather than wrap: a wrapped product could
+            DataspaceType::Simple => self
-                    // under-size a buffer. Size-critical callers use
+                .dimensions
-                    // `checked_num_elements`.
+                .iter()
-                    self.dimensions
+                .fold(1u64, |acc, &d| acc.saturating_mul(d)),
-                        .iter()
-                        .fold(1u64, |acc, &d| acc.saturating_mul(d))
-                }
-            }
         }
     }
 }
@@ -352,4 +366,44 @@ mod tests {
         let ds = Dataspace::parse(&data, 8).unwrap();
         assert_eq!(ds.max_dimensions, Some(vec![10]));
     }
+
+    /// A simple dataspace of rank 0 (cve-2020-18494's `/dset1`) holds one
+    /// element in libhdf5, which h5py reads as shape `()`. It was 0.
+    #[test]
+    fn simple_rank_zero_holds_one_element() {
+        let data = build_v2_dataspace(0, 0, 1, &[], None);
+        let ds = Dataspace::parse(&data, 8).unwrap();
+        assert_eq!(ds.space_type, DataspaceType::Simple);
+        assert_eq!(ds.num_elements(), 1);
+        assert_eq!(ds.checked_num_elements().unwrap(), 1);
+    }
+
+    /// `H5O__sdspace_decode`'s checks.
+    #[test]
+    fn refuses_what_libhdf5_refuses() {
+        let too_many = build_v2_dataspace(33, 0, 1, &[1; 33], None);
+        assert!(matches!(
+            Dataspace::parse(&too_many, 8),
+            Err(FormatError::InvalidDataspace(_))
+        ));
+        let scalar_with_rank = build_v2_dataspace(1, 0, 0, &[4], None);
+        assert!(matches!(
+            Dataspace::parse(&scalar_with_rank, 8),
+            Err(FormatError::InvalidDataspace(_))
+        ));
+        let null_with_rank = build_v2_dataspace(1, 0, 2, &[4], None);
+        assert!(matches!(
+            Dataspace::parse(&null_with_rank, 8),
+            Err(FormatError::InvalidDataspace(_))
+        ));
+        let over_max = build_v1_dataspace(2, 0x01, &[5, 20], Some(&[10, 10]));
+        assert!(matches!(
+            Dataspace::parse(&over_max, 8),
+            Err(FormatError::InvalidDataspace(_))
+        ));
+        // 32 dimensions, and a size equal to the maximum or unlimited, are fine.
+        assert!(Dataspace::parse(&build_v2_dataspace(32, 0, 1, &[1; 32], None), 8).is_ok());
+        let at_max = build_v1_dataspace(2, 0x01, &[10, 20], Some(&[10, u64::MAX]));
+        assert!(Dataspace::parse(&at_max, 8).is_ok());
+    }
 }

crates/clawhdf5-format/src/error.rs

@@ -226,6 +226,10 @@ pub enum FormatError {
     /// A link libhdf5 refuses to list: a symbol-table entry with an empty
     /// name ("invalid link name"). Listing the group fails, as in libhdf5.
     InvalidLinkName,
+    /// A dataspace message libhdf5 refuses to decode (the reason is
+    /// libhdf5's own error text): more than 32 dimensions, a rank on a
+    /// scalar or null dataspace, a dimension larger than its maximum.
+    InvalidDataspace(&'static str),
 }
 
 impl fmt::Display for FormatError {
@@ -500,6 +504,9 @@ impl fmt::Display for FormatError {
             FormatError::InvalidLinkName => {
                 write!(f, "invalid link name: a group entry has an empty name")
             }
+            FormatError::InvalidDataspace(why) => {
+                write!(f, "invalid dataspace: {why}")
+            }
         }
     }
 }

crates/clawhdf5/src/mmap_file.rs

@@ -390,13 +390,7 @@ impl<'f> MmapDataset<'f> {
         match &dl {
             DataLayout::Contiguous { address, size } => {
                 let addr = address.ok_or(Error::Format(FormatError::NoDataAllocated))?;
-                let sz = *size as usize;
+                let sz = clawhdf5_format::data_read::contiguous_read_len(*size, expected)?;
-                if sz != expected {
-                    return Err(Error::Format(FormatError::DataSizeMismatch {
-                        expected,
-                        actual: sz,
-                    }));
-                }
                 let data = self.file.hdf5_bytes();
                 let a = addr as usize;
                 if a + sz > data.len() {