fix: open a file whose cache image cannot load, and fail its objects
For a metadata cache image libhdf5 cannot load, libhdf5 opens the file and fails the first metadata read (the image loads on the first H5C_protect after open); h5py reports the error on the root group. The conformance probe reported it that way, but File::open refused the file, so the gate counted cve-2025-6269-1..4 and cve-2025-6516 as agreeing with h5py for behaviour the library did not have. The library now behaves as the probe reports: File (mmap, buffered and from_bytes) and MmapFile open the file and every object lookup (dataset, dataset_at, group, group listings and attributes, VL decoding) fails with the image's error; LazyFile reads the root group's header at open, so its open is that first read and fails. Probe and library take the three-way decision (refuse at open / image loads / image cannot load) from the same clawhdf5_format::superblock_ext::cache_image_state. One deliberate difference from libhdf5 remains, documented: after the failed first read libhdf5 reads the file's own metadata, which the image was meant to replace and may be stale; here every lookup keeps failing. File::cache_image_error / MmapFile::cache_image_error expose the error to code that parses as_bytes() itself; h5rs checks it before reading any object header (h5rs ls on cve-2025-6269-1 said "invalid object header version: 0" from the stale bytes). Test: metadata_cache_image.rs an_image_libhdf5_cannot_load_fails_every_object (the fixture with its image signature broken; h5py opens that file and fails the first read with "Bad metadata cache image header signature"). It fails on the previous commit, where File::open refuses the file. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -234,6 +234,11 @@ impl H5 {
|
||||
}
|
||||
|
||||
pub fn header(&self, addr: u64) -> Result<ObjectHeader> {
|
||||
// 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}")))
|
||||
|
||||
@@ -98,6 +98,9 @@ impl<R: HDF5Read> LazyFile<R> {
|
||||
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 {
|
||||
|
||||
@@ -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<clawhdf5_io::PrivateCopy>,
|
||||
/// The file has a metadata cache image libhdf5 cannot load: every
|
||||
/// object lookup fails with this error (see `File`).
|
||||
image_error: Option<FormatError>,
|
||||
}
|
||||
|
||||
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<MmapDataset<'_>, 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<MmapGroup<'_>, 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, FormatError> {
|
||||
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<Vec<GroupEntry>, 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)
|
||||
}
|
||||
|
||||
@@ -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<clawhdf5_io::PrivateCopy>,
|
||||
/// 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<FormatError>,
|
||||
}
|
||||
|
||||
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<Dataset<'_>, 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<Group<'_>, 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<Vec<Vec<u8>>, 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<Vec<Vec<T>>, 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, FormatError> {
|
||||
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<Vec<GroupEntry>, 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)
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user