fix: apply the superblock extension and cache image in every opener

File, MmapFile and LazyFile decoded the superblock extension and laid a
metadata cache image over the file's metadata; the other readers did
not, so the same file read differently by entry point: NativeVol,
AsyncHDF5File and MpiVol (clawhdf5-io) and the external source files of
a virtual dataset (clawhdf5-format vds.rs) read a file with an image
from its own bytes, which libhdf5 does not (they may be stale, or zeros:
h5clear_mdc_image.h5 failed with InvalidObjectHeaderVersion(0)), and
skipped the extension checks File::open makes (cve-2020-10810/10812).

Each of them owns its buffer, so each now calls the shared
superblock_ext::apply_cache_image_in_place, which checks the extension
and writes the image's entries in place (only the image block is
copied). These readers read whole datasets and cannot open a file and
fail each object, so an image libhdf5 cannot load is refused with the
image's error, never read around. clawhdf5-io's vol::load_hdf5 wraps it
for NativeVol (at open; for from_bytes the error is reported on read,
as a truncated file already was) and MpiVol. The MpiVol edit is minimal
and was not compiled: the mpi-io feature needs an MPI installation this
machine does not have (mpi-sys's build script panics).

Tests: NativeVol (open_path and from_bytes), AsyncHDF5File and a VDS
whose source file is h5clear_mdc_image.h5 (vds_interop.rs, against
h5py) read the fixture's values; the corrupted-image variants are
refused. Each fails without its fix.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-26 11:49:13 -05:00
co-authored by Claude Opus 5.5
parent 6559a91495
commit d493d4792e
6 changed files with 176 additions and 6 deletions
+12
View File
@@ -34,6 +34,18 @@ ZFP filters; the 2 mismatches are the known h5py big-endian VL bug.
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`.
- **Every other opener applies the superblock extension and the cache
image too** (`superblock_ext::apply_cache_image_in_place`, writing into
the buffer each already owns): `clawhdf5_io`'s `NativeVol` (at `open`,
and on read for `from_bytes`), `AsyncHDF5File`, `MpiVol` (a minimal edit
through the same `vol::load_hdf5`; the `mpi-io` feature cannot be built
without an MPI installation, so it was not compiled), and the external
source files of a virtual dataset. They read a file with an image from
its own bytes — stale metadata, or none (`h5clear_mdc_image.h5` failed
with `InvalidObjectHeaderVersion(0)`) — and skipped the extension checks
`File::open` makes. These readers cannot open a file and fail each
object, so an image libhdf5 cannot load is refused with the image's
error.
- **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).
+22 -1
View File
@@ -803,7 +803,11 @@ impl<'a, 'r> Sources<'a, 'r> {
let resolver = self.resolver.ok_or_else(|| {
vds_err("external-file virtual dataset sources require a file resolver")
})?;
self.cached_file = Some((String::from(name), resolver(name)?));
let mut bytes = resolver(name)?;
if let Some(b) = bytes.as_mut() {
load_source_file(b)?;
}
self.cached_file = Some((String::from(name), bytes));
}
// An external file is handed over whole; its addresses are relative
// to its superblock, so skip any user block.
@@ -851,6 +855,23 @@ impl<'a, 'r> Sources<'a, 'r> {
}
}
/// Check an external source file's superblock extension as libhdf5 does
/// when it opens the file, and write any metadata cache image over its
/// metadata in place: libhdf5 reads the image's entries instead of the
/// file's own, possibly stale, bytes (`crate::superblock_ext`). A source
/// file whose image cannot be loaded is an error, as other corrupt source
/// files are here.
fn load_source_file(whole: &mut [u8]) -> Result<(), FormatError> {
let base = crate::signature::find_signature(whole)?;
let sb = crate::superblock::Superblock::parse(&whole[base..], 0)?;
// The end of file the superblock records; a truncated source file is
// read as before, up to its length.
let end = sb
.data_end(base as u64, whole.len() as u64)
.map_or(whole.len(), |e| base + e as usize);
crate::superblock_ext::apply_cache_image_in_place(&mut whole[base..end], &sb)
}
/// Whether elements of `dt` contain addresses into their own file:
/// variable-length data (global-heap IDs) or references.
fn holds_file_addresses(dt: &Datatype) -> bool {
+41
View File
@@ -288,6 +288,12 @@ impl AsyncHDF5File {
// superblock records, as libhdf5 does.
let end = superblock.data_end(user_block as u64, whole_len)?;
data.truncate(end as usize);
// Check the superblock extension as libhdf5 does at open, and write
// any metadata cache image over the file's metadata (libhdf5 reads
// the image's entries instead of the file's own, possibly stale,
// bytes). An image libhdf5 cannot load is refused: this reader has
// no way to open the file and fail each object instead.
clawhdf5_format::superblock_ext::apply_cache_image_in_place(&mut data, &superblock)?;
Ok(Self { data, superblock })
}
@@ -430,6 +436,41 @@ mod tests {
fw.finish().unwrap()
}
/// libhdf5's `h5clear_mdc_image.h5` (see the clawhdf5 crate's
/// `tests/metadata_cache_image.rs`): the root group's header exists
/// only in the file's metadata cache image.
fn cache_image_fixture() -> Vec<u8> {
std::fs::read(concat!(
env!("CARGO_MANIFEST_DIR"),
"/../clawhdf5/tests/fixtures/h5clear_mdc_image.h5"
))
.unwrap()
}
#[tokio::test]
async fn reads_through_a_metadata_cache_image() {
let bytes = cache_image_fixture();
let file = AsyncHDF5File::from_bytes(bytes.clone()).unwrap();
let info = file.read_dataset_raw("DSET").await.unwrap();
assert_eq!(info.shape, [50, 100]);
let values: Vec<i32> = info
.raw
.chunks_exact(4)
.map(|b| i32::from_le_bytes(b.try_into().unwrap()))
.collect();
let expected: Vec<i32> = (0..50).flat_map(|i| (0..100).map(move |j| i * j)).collect();
assert_eq!(values, expected);
// An image libhdf5 cannot load is refused at open.
let mut bad = bytes;
let at = bad.windows(4).position(|w| w == b"MDCI").unwrap();
bad[at] = b'X';
assert!(matches!(
AsyncHDF5File::from_bytes(bad),
Err(AsyncHDF5Error::Format(FormatError::InvalidCacheImage(_)))
));
}
// --- AsyncMemoryReader tests ---
#[tokio::test]
+4 -1
View File
@@ -191,7 +191,10 @@ fn mpi_collective_read(vol: &MpiVol, location: &str, path: &str) -> Result<Vec<u
let mut len_buf = [0usize; 1];
if rank == 0 {
let file = std::fs::read(location).map_err(VolError::Io)?;
let mut file = std::fs::read(location).map_err(VolError::Io)?;
// Checked as libhdf5 checks a file at open, with any metadata cache
// image written over the metadata (see `vol::load_hdf5`).
crate::vol::load_hdf5(&mut file)?;
// From the superblock to the recorded end of file; truncated files
// are refused.
let (bytes, sb) = crate::vol::hdf5_view(&file)?;
+74 -4
View File
@@ -208,6 +208,9 @@ pub trait VirtualObjectLayer: Send + Sync {
pub struct NativeVol {
data: Option<Vec<u8>>,
location: Option<String>,
/// Why the bytes given to [`NativeVol::from_bytes`] cannot be read
/// (what `open` would have refused them for).
load_error: Option<String>,
}
/// The HDF5 bytes of a whole file and its superblock: from the superblock
@@ -228,12 +231,35 @@ pub(crate) fn hdf5_view(
Ok((&data[..end as usize], sb))
}
/// Check a whole file as libhdf5 does when it opens it ([`hdf5_view`], and
/// the superblock extension), and write any metadata cache image over the
/// file's metadata in place: libhdf5 reads the image's entries instead of
/// the file's own bytes at their addresses, which may be stale
/// (`clawhdf5_format::superblock_ext`). A file whose image libhdf5 cannot
/// load is refused: a connector that reads whole datasets has no way to
/// open the file and fail each object instead.
pub(crate) fn load_hdf5(whole: &mut [u8]) -> Result<(), VolError> {
use clawhdf5_format::superblock_ext::apply_cache_image_in_place;
let err = |e: clawhdf5_format::error::FormatError| VolError::DataError(e.to_string());
let (len, sb) = {
let (data, sb) = hdf5_view(whole)?;
(data.len(), sb)
};
// hdf5_view's bytes start at the superblock, after any user block.
let base = clawhdf5_format::signature::split_user_block(whole)
.map_err(err)?
.0
.len();
apply_cache_image_in_place(&mut whole[base..base + len], &sb).map_err(err)
}
impl NativeVol {
/// Create a new native VOL connector.
pub fn new() -> Self {
Self {
data: None,
location: None,
load_error: None,
}
}
@@ -245,10 +271,12 @@ impl NativeVol {
}
/// Create a native VOL connector from bytes already in memory.
pub fn from_bytes(data: Vec<u8>) -> Self {
pub fn from_bytes(mut data: Vec<u8>) -> Self {
let load_error = load_hdf5(&mut data).err().map(|e| e.to_string());
Self {
data: Some(data),
location: Some("<memory>".into()),
load_error,
}
}
@@ -281,10 +309,12 @@ impl VirtualObjectLayer for NativeVol {
}
fn open(&mut self, location: &str) -> Result<(), VolError> {
let data = std::fs::read(location)?;
// Refuse a truncated file at open, as libhdf5 does.
hdf5_view(&data)?;
let mut data = std::fs::read(location)?;
// Refuse a truncated file at open, as libhdf5 does, and load any
// metadata cache image.
load_hdf5(&mut data)?;
self.data = Some(data);
self.load_error = None;
self.location = Some(location.to_string());
Ok(())
}
@@ -299,6 +329,9 @@ impl VirtualObjectLayer for NativeVol {
let data = self.data.as_ref().ok_or_else(|| {
VolError::Io(io::Error::new(io::ErrorKind::NotConnected, "file not open"))
})?;
if let Some(e) = &self.load_error {
return Err(VolError::DataError(e.clone()));
}
use clawhdf5_format::{
data_layout::DataLayout, data_read::read_raw_data_full, dataspace::Dataspace,
@@ -434,6 +467,43 @@ mod tests {
assert_eq!(raw.len(), 24);
}
/// libhdf5's `h5clear_mdc_image.h5` (see the clawhdf5 crate's
/// `tests/metadata_cache_image.rs`): the root group's header exists
/// only in the file's metadata cache image, so reading the file's own
/// bytes finds zeros there.
#[test]
fn native_vol_reads_through_a_metadata_cache_image() {
let path = concat!(
env!("CARGO_MANIFEST_DIR"),
"/../clawhdf5/tests/fixtures/h5clear_mdc_image.h5"
);
let expected: Vec<u8> = (0..50)
.flat_map(|i| (0..100).map(move |j| i * j))
.flat_map(i32::to_le_bytes)
.collect();
let vol = NativeVol::open_path(path).unwrap();
assert_eq!(vol.read_dataset("DSET").unwrap(), expected);
let bytes = std::fs::read(path).unwrap();
let vol = NativeVol::from_bytes(bytes.clone());
assert_eq!(vol.read_dataset("DSET").unwrap(), expected);
// An image libhdf5 cannot load is refused, not read around.
let mut bad = bytes;
let at = bad.windows(4).position(|w| w == b"MDCI").unwrap();
bad[at] = b'X';
let err = NativeVol::from_bytes(bad.clone())
.read_dataset("DSET")
.unwrap_err();
assert!(err.to_string().contains("cache image"), "{err}");
let dir = tempfile::tempdir().unwrap();
let bad_path = dir.path().join("bad_image.h5");
std::fs::write(&bad_path, &bad).unwrap();
let err = NativeVol::open_path(bad_path.to_str().unwrap())
.err()
.unwrap();
assert!(err.to_string().contains("cache image"), "{err}");
}
#[test]
fn vol_error_display() {
let err = VolError::Unsupported("read_dataset".into());
+23
View File
@@ -486,3 +486,26 @@ fn vds_libhdf5_test_files() {
vec![5, 10, 10]
);
}
/// A source file with a metadata cache image (libhdf5's
/// `h5clear_mdc_image.h5`, whose root group's header exists only in the
/// image) is read through the image, as libhdf5 opens it. Source files were
/// read from their own bytes, and this one failed on the root group.
#[test]
fn vds_source_file_with_a_metadata_cache_image() {
skip_if_no_python!();
let dir = tempfile::tempdir().unwrap();
let fixture = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/h5clear_mdc_image.h5");
std::fs::copy(&fixture, dir.path().join("src.h5")).unwrap();
generate(
dir.path(),
r#"
with h5py.File("vds.h5", "w", libver="latest") as f:
lay = h5py.VirtualLayout(shape=(3, 100), dtype="i4")
lay[0:3, :] = h5py.VirtualSource("src.h5", "DSET", shape=(50, 100))[5:8, :]
f.create_virtual_dataset("v", lay)
expect("vds.h5", "v", "image_source")
"#,
);
assert_matches_libhdf5(dir.path(), "vds.h5", "v", "image_source");
}