diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b0efd6..d256a15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,8 +24,16 @@ ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. image, and an abort for an 8 GiB one; `tests/cache_image_memory.rs` guards it.) An image entry that runs past the end of file is refused (libhdf5 checks only its start; the images it writes never do this). A - file whose image libhdf5 cannot load opens in libhdf5 but nothing in it - can be read; `File::open` refuses it. + file whose image libhdf5 cannot load (`cve-2025-6269-*`, `cve-2025-6516`) + opens, as in libhdf5, and every object lookup fails with the image's + error (`File`, `MmapFile`; `LazyFile` reads the root group at open, so + its open fails). libhdf5 fails only its first metadata read and then + reads the file's own, possibly stale, bytes; those are never read here. + An interim version refused such a file at `File::open` while the + conformance probe reported it as libhdf5 does, so the gate counted five + files as agreeing with h5py that the library did not open; probe and + library now take the decision from the same + `superblock_ext::cache_image_state`. - **The superblock extension is decoded at open, as libhdf5 does:** a File Space Info or Metadata Cache Image message libhdf5 cannot decode makes the open fail (`cve-2020-10810`, `cve-2020-10812` were opened). diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 8c25a88..71c3139 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -712,12 +712,14 @@ fn main() { return; } }; - // libhdf5 decodes the superblock extension at open (File::open does - // the same), and loads a metadata cache image over the file's own + // libhdf5 decodes the superblock extension at open (an error refuses + // the file), and loads a metadata cache image over the file's own // metadata. It loads the image only when it first reads metadata — the // root group — so a file whose image it cannot load still opens and - // every object fails; File::open refuses such a file outright. The - // probe records the image's error where libhdf5 reports it. + // that read fails. The library decides all three cases with the same + // `cache_image_state`: `File` and `MmapFile` open such a file and fail + // every object lookup with the image's error, which is what the probe + // records here (on the root group, where libhdf5 reports it). use clawhdf5_format::superblock_ext::{self, CacheImageState}; let state = match guarded(|| superblock_ext::cache_image_state(hdf5, &sb).map_err(e)) { Ok(x) => x, diff --git a/crates/clawhdf5-tools/src/h5.rs b/crates/clawhdf5-tools/src/h5.rs index 1f4354a..5f71a14 100644 --- a/crates/clawhdf5-tools/src/h5.rs +++ b/crates/clawhdf5-tools/src/h5.rs @@ -234,6 +234,11 @@ impl H5 { } pub fn header(&self, addr: u64) -> Result { + // A metadata cache image libhdf5 cannot load: the file opens, and + // every object fails (its bytes may hold stale metadata). + if let Some(e) = self.file.cache_image_error() { + return Err(Error::at(addr, format!("metadata cache image: {e}"))); + } let off = usize::try_from(addr).map_err(|_| Error::at(addr, "address out of range"))?; ObjectHeader::parse(self.data(), off, self.os(), self.ls()) .map_err(|e| Error::at(addr, format!("object header: {e}"))) diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index cb688e9..34f46cc 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -98,6 +98,9 @@ impl LazyFile { let patched = match view { crate::cache_image::ImageView::Plain => None, crate::cache_image::ImageView::Patched(p) => Some(p), + // libhdf5 opens such a file and fails its first metadata read; + // a LazyFile reads the root group's header at open, so the open + // is that read. crate::cache_image::ImageView::Unloadable(e) => return Err(e.into()), }; let data = match &patched { diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 0de5b02..040c02f 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -43,6 +43,9 @@ pub struct MmapFile { /// the image's entries land on are copied (see `crate::cache_image`). /// `None` for a file without an image, read straight from the mapping. patched: Option, + /// The file has a metadata cache image libhdf5 cannot load: every + /// object lookup fails with this error (see `File`). + image_error: Option, } impl MmapFile { @@ -61,10 +64,10 @@ impl MmapFile { crate::cache_image::private_view(reader.as_bytes(), base, end, &superblock, || { clawhdf5_io::HDF5Read::private_copy(&reader) })?; - let patched = match view { - crate::cache_image::ImageView::Plain => None, - crate::cache_image::ImageView::Patched(p) => Some(p), - crate::cache_image::ImageView::Unloadable(e) => return Err(e.into()), + let (patched, image_error) = match view { + crate::cache_image::ImageView::Plain => (None, None), + crate::cache_image::ImageView::Patched(p) => (Some(p), None), + crate::cache_image::ImageView::Unloadable(e) => (None, Some(e)), }; Ok(Self { reader, @@ -72,6 +75,7 @@ impl MmapFile { end, superblock, patched, + image_error, }) } @@ -99,7 +103,7 @@ impl MmapFile { /// Resolve a path and return a `MmapDataset` handle. pub fn dataset(&self, path: &str) -> Result, Error> { - let data = self.hdf5_bytes(); + let data = self.meta()?; let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; let hdr = self.parse_header(addr)?; if !has_message(&hdr, MessageType::DataLayout) { @@ -114,7 +118,7 @@ impl MmapFile { /// Resolve a path and return a `MmapGroup` handle. pub fn group(&self, path: &str) -> Result, Error> { - let data = self.hdf5_bytes(); + let data = self.meta()?; let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; Ok(MmapGroup { file: self, @@ -129,14 +133,29 @@ impl MmapFile { self.hdf5_bytes() } + /// The error of a metadata cache image libhdf5 cannot load, when the + /// file has one (see [`crate::File::cache_image_error`]). + pub fn cache_image_error(&self) -> Option<&FormatError> { + self.image_error.as_ref() + } + /// Returns a reference to the parsed superblock. pub fn superblock(&self) -> &Superblock { &self.superblock } + /// The bytes to read metadata from; fails for a file whose cache image + /// cannot be loaded. + fn meta(&self) -> Result<&[u8], FormatError> { + match &self.image_error { + Some(e) => Err(e.clone()), + None => Ok(self.hdf5_bytes()), + } + } + fn parse_header(&self, address: u64) -> Result { ObjectHeader::parse( - self.hdf5_bytes(), + self.meta()?, address as usize, self.superblock.offset_size, self.superblock.length_size, @@ -257,7 +276,7 @@ impl<'f> MmapGroup<'f> { /// [`group_v2::resolve_group_children`]); dangling, external and /// user-defined links are left out. fn children(&self) -> Result, Error> { - let data = self.file.hdf5_bytes(); + let data = self.file.meta()?; group_v2::resolve_group_children(data, &self.file.superblock, self.address) .map_err(Error::Format) } diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 9f6da0f..023e417 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -63,6 +63,11 @@ struct FileData { /// an image is read straight from the mapping, and an owned buffer has /// the image written into it in place. patched: Option, + /// The file has a metadata cache image libhdf5 cannot load. libhdf5 + /// opens such a file and fails its first metadata read (the image loads + /// then); every object lookup here fails with this error, and no + /// metadata is read from the file's own, possibly stale, bytes. + image_error: Option, } impl FileData { @@ -88,10 +93,10 @@ impl FileData { })? } }; - let patched = match view { - ImageView::Plain => None, - ImageView::Patched(p) => Some(p), - ImageView::Unloadable(e) => return Err(e.into()), + let (patched, image_error) = match view { + ImageView::Plain => (None, None), + ImageView::Patched(p) => (Some(p), None), + ImageView::Unloadable(e) => (None, Some(e)), }; Ok(( Self { @@ -99,6 +104,7 @@ impl FileData { base, end, patched, + image_error, }, superblock, )) @@ -114,6 +120,15 @@ impl FileData { fn len(&self) -> usize { self.as_bytes().len() } + + /// The bytes to read metadata from; fails for a file whose cache image + /// cannot be loaded (see [`Self::image_error`]). + fn meta(&self) -> Result<&[u8], FormatError> { + match &self.image_error { + Some(e) => Err(e.clone()), + None => Ok(self.as_bytes()), + } + } } // --------------------------------------------------------------------------- @@ -200,7 +215,7 @@ impl File { /// /// The path uses `/` separators (e.g., `"group1/values"`). pub fn dataset(&self, path: &str) -> Result, Error> { - let data = self.data.as_bytes(); + let data = self.data.meta()?; let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; let hdr = self.parse_header(addr)?; if !has_message(&hdr, MessageType::DataLayout) { @@ -235,7 +250,7 @@ impl File { /// The path uses `/` separators (e.g., `"sensors"`). /// Use `"/"` or `""` for the root group. pub fn group(&self, path: &str) -> Result, Error> { - let data = self.data.as_bytes(); + let data = self.data.meta()?; let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; Ok(Group { file: self, @@ -291,11 +306,23 @@ impl File { /// Returns the file's bytes from the superblock on (after any user /// block). Every HDF5 address in the file indexes this slice, so it is - /// what the `clawhdf5_format` parsers expect as `file_data`. + /// what the `clawhdf5_format` parsers expect as `file_data`. For a file + /// with a metadata cache image these are the bytes with the image + /// applied; when the image cannot be loaded they are the file's own + /// bytes, whose metadata may be stale (every object lookup fails then). pub fn as_bytes(&self) -> &[u8] { self.data.as_bytes() } + /// The error of a metadata cache image libhdf5 cannot load, when the + /// file has one. Such a file opens, as in libhdf5, and every object + /// lookup fails with this error; code that parses [`Self::as_bytes`] + /// itself should check it first, since those bytes then hold the + /// file's own, possibly stale, metadata. + pub fn cache_image_error(&self) -> Option<&FormatError> { + self.data.image_error.as_ref() + } + /// Size of the user block before the superblock (0 for most files). /// Matches h5py's `File.userblock_size`. pub fn user_block_size(&self) -> u64 { @@ -340,7 +367,7 @@ impl File { raw: &[u8], ) -> Result>, Error> { crate::vlen::decode_string_bytes( - self.as_bytes(), + self.data.meta()?, datatype, raw, self.offset_size(), @@ -358,7 +385,7 @@ impl File { raw: &[u8], ) -> Result>, Error> { crate::vlen::decode_vlen( - self.as_bytes(), + self.data.meta()?, datatype, raw, self.offset_size(), @@ -368,7 +395,7 @@ impl File { fn parse_header(&self, address: u64) -> Result { ObjectHeader::parse( - self.data.as_bytes(), + self.data.meta()?, address as usize, self.superblock.offset_size, self.superblock.length_size, @@ -490,7 +517,7 @@ impl<'f> Group<'f> { /// [`group_v2::resolve_group_children`]); dangling, external and /// user-defined links are left out. fn children(&self) -> Result, Error> { - let data = self.file.data.as_bytes(); + let data = self.file.data.meta()?; group_v2::resolve_group_children(data, &self.file.superblock, self.address) .map_err(Error::Format) } diff --git a/crates/clawhdf5/tests/metadata_cache_image.rs b/crates/clawhdf5/tests/metadata_cache_image.rs index 3cbc496..92463e8 100644 --- a/crates/clawhdf5/tests/metadata_cache_image.rs +++ b/crates/clawhdf5/tests/metadata_cache_image.rs @@ -46,3 +46,49 @@ fn mmap_and_lazy_files_read_through_the_cache_image() { expected() ); } + +/// A file whose cache image libhdf5 cannot load: libhdf5 opens it, and the +/// first metadata read fails with the image's error (h5py: +/// `Unable to get group info (Bad metadata cache image header signature)`); +/// later reads get the file's own bytes, which here are stale (the root +/// group's header is zeros: "bad object header version number"). The +/// openers agree on the open and the failure: `File` and `MmapFile` open +/// and fail every object lookup with the image's error (never reading the +/// stale bytes), `LazyFile` reads the root group's header at open and so +/// fails there. `File::open` used to refuse the file, while the +/// conformance probe reported it as h5py does. +#[test] +fn an_image_libhdf5_cannot_load_fails_every_object() { + let mut bytes = std::fs::read(fixture()).unwrap(); + let at = bytes.windows(4).position(|w| w == b"MDCI").unwrap(); + bytes[at] = b'X'; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("bad_image.h5"); + std::fs::write(&path, &bytes).unwrap(); + + let is_image_error = |e: clawhdf5::Error| { + matches!( + e, + clawhdf5::Error::Format(clawhdf5_format::error::FormatError::InvalidCacheImage( + "bad metadata cache image header signature" + )) + ) + }; + for file in [ + File::open(&path).unwrap(), + File::from_bytes(bytes.clone()).unwrap(), + ] { + assert!(is_image_error(file.root().datasets().unwrap_err())); + assert!(is_image_error(file.root().attrs().unwrap_err())); + assert!(is_image_error(file.dataset("DSET").unwrap_err())); + assert!(is_image_error(file.group("/").map(|_| ()).unwrap_err())); + assert!(is_image_error(file.dataset_at(96).unwrap_err())); + } + let mm = MmapFile::open(&path).unwrap(); + assert!(is_image_error(mm.root().datasets().unwrap_err())); + assert!(is_image_error(mm.dataset("DSET").unwrap_err())); + assert!(is_image_error(mm.group("/").map(|_| ()).unwrap_err())); + assert!(is_image_error( + LazyFile::from_bytes(bytes).map(|_| ()).unwrap_err() + )); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index b7c471a..8328f14 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -205,9 +205,19 @@ fill-value item that did is fixed). - Metadata cache images are not supported. **Fixed 2026-09-26:** the image is applied at open, as libhdf5 loads it over the file's metadata (`clawhdf5_format::superblock_ext`); `h5clear_mdc_image.h5` reads - (`crates/clawhdf5/tests/metadata_cache_image.rs`). A file whose image - libhdf5 cannot load (`cve-2025-6269-*`, `cve-2025-6516`) opens in - libhdf5 with nothing readable in it; `File::open` refuses it. + (`crates/clawhdf5/tests/metadata_cache_image.rs`), without copying the + file (a private copy-on-write mapping takes the image's entries; + `tests/cache_image_memory.rs`). A file whose image libhdf5 cannot load + (`cve-2025-6269-*`, `cve-2025-6516`) opens, as in libhdf5, and every + object lookup fails with the image's error. Differences from libhdf5 + that remain: libhdf5 fails only the first metadata read and then reads + the file's own (possibly stale) metadata, where we keep failing; an + image entry that runs past the end of file is refused (libhdf5 checks + only its start); a flush-dependency parent flag is checked against the + child count as libhdf5's debug build checks it (HDF5 2.0 release + builds refuse every entry that has children, even in images they + wrote); the superblock extension's driver-info and shared-message table + messages are not decoded at open. - x87 long double and binary128 are refused. - N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail. **Not our bug (checked 2026-09-26):** both are corrupt files HDF5 2.0