From b22b15f00a868e3e4fa48c70ebc636eaac3944b5 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:22:04 -0500 Subject: [PATCH] fix: refuse at open the dataset storage libhdf5 refuses at open libhdf5 checks a dataset's storage when it opens the dataset (H5D__contig_check, H5D__compact_init): the element count times the element size must not overflow, contiguous storage must end inside the file, compact data must be the dataset's size. File::dataset opened cve-2024-32624's /Dset_OBJREF (2^62 + 2 references of 8 bytes) and reported its shape; only reading failed. data_read::check_dataset_storage makes those checks (new FormatError::InvalidDatasetStorage), and File, MmapFile and LazyFile run it whenever they open a dataset (by path, by address, from a group), as does the conformance probe. As before, a datatype, dataspace or layout that does not decode is left for the read to report, so such a dataset still opens and its attributes still read. An empty contiguous dataset at a defined address, which libhdf5 refuses, is still accepted: clawhdf5 up to v2.7.0 wrote them. Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/probe/src/main.rs | 14 +-- crates/clawhdf5-format/src/data_read.rs | 93 +++++++++++++++++++ crates/clawhdf5-format/src/error.rs | 8 ++ crates/clawhdf5/src/lazy.rs | 28 +++++- crates/clawhdf5/src/mmap_file.rs | 28 +++++- crates/clawhdf5/src/reader.rs | 33 +++++-- .../tests/header_validation_interop.rs | 49 ++++++++++ 7 files changed, 233 insertions(+), 20 deletions(-) diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index aa3d6a1..844f343 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -308,18 +308,20 @@ impl<'a> Ctx<'a> { ) .map_err(e)?; } - let (shape, n) = Self::shape(&ds); - rec.insert("shape".into(), shape); - if n.saturating_mul(dt.type_size() as u64) > MAX_BYTES { - rec.insert("skipped".into(), Value::String("too large".into())); - return Ok(()); - } let lm = h .messages .iter() .find(|m| m.msg_type == MessageType::DataLayout) .ok_or("MissingMessage(DataLayout)")?; let dl = DataLayout::parse(&lm.data, self.os, self.ls).map_err(e)?; + // What libhdf5 checks when it opens the dataset (as File::dataset). + data_read::check_dataset_storage(&dl, &ds, &dt, self.data.len() as u64).map_err(e)?; + let (shape, n) = Self::shape(&ds); + rec.insert("shape".into(), shape); + if n.saturating_mul(dt.type_size() as u64) > MAX_BYTES { + rec.insert("skipped".into(), Value::String("too large".into())); + return Ok(()); + } rec.insert( "layout".into(), Value::String( diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index d1766a9..591142c 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -32,6 +32,64 @@ fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatErr Ok(()) } +/// The storage checks libhdf5 makes when it opens a dataset, before any +/// data is read (`H5D__contig_check`, `H5D__compact_init`), so a dataset +/// they refuse fails to open, as in libhdf5, instead of opening and +/// reporting a shape nothing can be read from: +/// +/// - the element count times the element size must not overflow 64 bits +/// ("size of dataset's storage overflowed" — `cve-2024-32624` +/// `/Dset_OBJREF`, 2^62 references of 8 bytes); +/// - contiguous storage at a defined address must end within the file's +/// `file_len` bytes (the HDF5 data up to the end of file the superblock +/// records); +/// - compact data must be exactly the dataset's size. +/// +/// Deliberately not refused, unlike libhdf5: an empty contiguous dataset at +/// a defined address (libhdf5's overflow test `addr + 0 <= addr` refuses +/// it), which clawhdf5 up to v2.7.0 wrote. Chunked and virtual layouts are +/// checked when their data is read. +pub fn check_dataset_storage( + layout: &DataLayout, + dataspace: &Dataspace, + datatype: &Datatype, + file_len: u64, +) -> Result<(), FormatError> { + if !matches!( + layout, + DataLayout::Contiguous { .. } | DataLayout::Compact { .. } + ) { + return Ok(()); + } + const OVERFLOWED: &str = "size of dataset's storage overflowed"; + let n = dataspace + .checked_num_elements() + .map_err(|_| FormatError::InvalidDatasetStorage(OVERFLOWED))?; + let data_size = n + .checked_mul(u64::from(datatype.type_size())) + .ok_or(FormatError::InvalidDatasetStorage(OVERFLOWED))?; + match layout { + DataLayout::Contiguous { + address: Some(address), + .. + } if address + .checked_add(data_size) + .is_none_or(|end| end > file_len) => + { + Err(FormatError::InvalidDatasetStorage( + "invalid dataset size, likely file corruption", + )) + } + DataLayout::Compact { data } if data.len() as u64 != data_size => { + Err(FormatError::InvalidDatasetStorage( + "bad value from dataset header - size of compact dataset's data buffer \ + doesn't match size of dataset data", + )) + } + _ => 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 @@ -2666,6 +2724,41 @@ mod tests { )); } + /// `H5D__contig_check` / `H5D__compact_init`, run when a dataset opens. + #[test] + fn dataset_storage_checks_at_open() { + let dt = make_f64_le_type(); + let contiguous = |address| DataLayout::Contiguous { address, size: 0 }; + // cve-2024-32624 `/Dset_OBJREF`: 2^62 + 2 elements of 8 bytes. + let huge = make_simple_dataspace(&[(1 << 62) + 2]); + assert_eq!( + check_dataset_storage(&contiguous(None), &huge, &dt, 1 << 20), + Err(FormatError::InvalidDatasetStorage( + "size of dataset's storage overflowed" + )) + ); + let ds = make_simple_dataspace(&[4]); + assert!(check_dataset_storage(&contiguous(Some(100)), &ds, &dt, 132).is_ok()); + assert!(matches!( + check_dataset_storage(&contiguous(Some(100)), &ds, &dt, 131), + Err(FormatError::InvalidDatasetStorage(_)) + )); + assert!(matches!( + check_dataset_storage(&contiguous(Some(u64::MAX - 8)), &ds, &dt, u64::MAX), + Err(FormatError::InvalidDatasetStorage(_)) + )); + // Not allocated, and (unlike libhdf5) empty at a defined address. + assert!(check_dataset_storage(&contiguous(None), &ds, &dt, 0).is_ok()); + let empty = make_simple_dataspace(&[0]); + assert!(check_dataset_storage(&contiguous(Some(64)), &empty, &dt, 64).is_ok()); + let compact = |n: usize| DataLayout::Compact { data: vec![0; n] }; + assert!(check_dataset_storage(&compact(32), &ds, &dt, 0).is_ok()); + assert!(matches!( + check_dataset_storage(&compact(24), &ds, &dt, 0), + Err(FormatError::InvalidDatasetStorage(_)) + )); + } + #[test] fn zerocopy_size_mismatch() { let dt = make_f64_le_type(); diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index b365794..ddd9ba7 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -230,6 +230,11 @@ pub enum FormatError { /// 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), + /// A dataset whose storage libhdf5 refuses when it opens the dataset + /// (the reason is libhdf5's own error text): an element count times + /// element size that overflows, contiguous storage past the end of the + /// file, compact data of the wrong size. + InvalidDatasetStorage(&'static str), } impl fmt::Display for FormatError { @@ -507,6 +512,9 @@ impl fmt::Display for FormatError { FormatError::InvalidDataspace(why) => { write!(f, "invalid dataspace: {why}") } + FormatError::InvalidDatasetStorage(why) => { + write!(f, "invalid dataset storage: {why}") + } } } } diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index a33b9bb..3232ee6 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -135,10 +135,11 @@ impl LazyFile { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(path.to_string())); } - Ok(LazyDataset { + LazyDataset { file: self, header: hdr, - }) + } + .check_open() } /// Resolve a path and return a `LazyGroup` handle. @@ -284,10 +285,11 @@ impl<'f, R: HDF5Read> LazyGroup<'f, R> { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(name.to_string())); } - Ok(LazyDataset { + LazyDataset { file: self.file, header: hdr, - }) + } + .check_open() } /// Get a subgroup within this group by name. @@ -326,6 +328,24 @@ pub struct LazyDataset<'f, R: HDF5Read> { } impl<'f, R: HDF5Read> LazyDataset<'f, R> { + /// libhdf5's storage checks when it opens a dataset + /// ([`data_read::check_dataset_storage`]): a dataset whose element count + /// times element size overflows, or whose contiguous storage runs past + /// the end of the file, fails to open. A datatype, dataspace or layout + /// that does not decode is left for the read to report, as before (the + /// dataset still opens, and its attributes can be read). + fn check_open(self) -> Result { + let decoded = (|| -> Result<_, Error> { + let data = self.required_payload(MessageType::Dataspace)?; + let ds = Dataspace::parse(&data, self.file.length_size())?; + Ok((self.datatype()?, ds, self.data_layout()?)) + })(); + if let Ok((dt, ds, dl)) = decoded { + data_read::check_dataset_storage(&dl, &ds, &dt, self.file.hdf5_bytes().len() as u64)?; + } + Ok(self) + } + /// Returns the shape (dimensions) of the dataset. pub fn shape(&self) -> Result, Error> { let ds = self.dataspace()?; diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 59adee9..2a8400b 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -85,10 +85,11 @@ impl MmapFile { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(path.to_string())); } - Ok(MmapDataset { + MmapDataset { file: self, header: hdr, - }) + } + .check_open() } /// Resolve a path and return a `MmapGroup` handle. @@ -211,10 +212,11 @@ impl<'f> MmapGroup<'f> { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(name.to_string())); } - Ok(MmapDataset { + MmapDataset { file: self.file, header: hdr, - }) + } + .check_open() } /// Get a subgroup within this group by name. @@ -253,6 +255,24 @@ pub struct MmapDataset<'f> { } impl<'f> MmapDataset<'f> { + /// libhdf5's storage checks when it opens a dataset + /// ([`data_read::check_dataset_storage`]): a dataset whose element count + /// times element size overflows, or whose contiguous storage runs past + /// the end of the file, fails to open. A datatype, dataspace or layout + /// that does not decode is left for the read to report, as before (the + /// dataset still opens, and its attributes can be read). + fn check_open(self) -> Result { + let decoded = (|| -> Result<_, Error> { + let data = self.required_payload(MessageType::Dataspace)?; + let ds = Dataspace::parse(&data, self.file.length_size())?; + Ok((self.datatype()?, ds, self.data_layout()?)) + })(); + if let Ok((dt, ds, dl)) = decoded { + data_read::check_dataset_storage(&dl, &ds, &dt, self.file.hdf5_bytes().len() as u64)?; + } + Ok(self) + } + /// Returns the shape (dimensions) of the dataset. pub fn shape(&self) -> Result, Error> { let ds = self.dataspace()?; diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 90faf11..40ac08b 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -170,10 +170,11 @@ impl File { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(path.to_string())); } - Ok(Dataset { + Dataset { file: self, header: hdr, - }) + } + .check_open() } /// A `Dataset` handle for the object header at `address` (an address @@ -186,10 +187,11 @@ impl File { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(format!("object at address {address}"))); } - Ok(Dataset { + Dataset { file: self, header: hdr, - }) + } + .check_open() } /// Resolve a path and return a `Group` handle. @@ -427,10 +429,11 @@ impl<'f> Group<'f> { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(name.to_string())); } - Ok(Dataset { + Dataset { file: self.file, header: hdr, - }) + } + .check_open() } /// Get a subgroup within this group by name. @@ -469,6 +472,24 @@ pub struct Dataset<'f> { } impl<'f> Dataset<'f> { + /// libhdf5's storage checks when it opens a dataset + /// ([`data_read::check_dataset_storage`]): a dataset whose element count + /// times element size overflows, or whose contiguous storage runs past + /// the end of the file, fails to open. A datatype, dataspace or layout + /// that does not decode is left for the read to report, as before (the + /// dataset still opens, and its attributes can be read). + fn check_open(self) -> Result { + let decoded = (|| -> Result<_, Error> { + let data = self.required_payload(MessageType::Dataspace)?; + let ds = Dataspace::parse(&data, self.file.length_size())?; + Ok((self.datatype()?, ds, self.data_layout()?)) + })(); + if let Ok((dt, ds, dl)) = decoded { + data_read::check_dataset_storage(&dl, &ds, &dt, self.file.data.len() as u64)?; + } + Ok(self) + } + /// Returns the shape (dimensions) of the dataset. pub fn shape(&self) -> Result, Error> { let ds = self.dataspace()?; diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs index 2f52d27..0bfd30e 100644 --- a/crates/clawhdf5/tests/header_validation_interop.rs +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -508,3 +508,52 @@ for libver in ("earliest", "latest"): ); } } + +/// cve-2024-32624 `/Dset_OBJREF`: a dataspace whose element count times the +/// element size overflows 64 bits. libhdf5 refuses to open the dataset +/// ("size of dataset's storage overflowed"); `File::dataset` used to open it +/// and report its shape, and only reading failed. The same for storage that +/// runs past the end of the file ("invalid dataset size, likely file +/// corruption"). +#[test] +fn dataset_storage_libhdf5_refuses_at_open_is_refused_at_open() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + // A contiguous int64 dataset of 3 elements, libver earliest (a version 1 + // dataspace holding the size and the maximum size). "overflow" makes + // both 2^62 + 2 (x 8 bytes overflows); "pasteof" makes both 1000 (the + // storage runs past the end of the file). + let verdicts = h5py_verdicts( + dir.path(), + r#" +good = os.path.join(d, "good.h5") +with h5py.File(good, "w", libver="earliest") as f: + f.create_dataset("d", data=np.array([7, 8, 9], dtype=" 0 and data.find(three, at + 1) < 0 +for name, n in (("overflow", (1 << 62) + 2), ("pasteof", 1000)): + bad = bytearray(data) + bad[at:at + 16] = struct.pack("