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:
@@ -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 {
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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)?;
|
||||
|
||||
@@ -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());
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user