From 256e7b89e43eb2a8294f8404f1e950274a43f0b8 Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 22:42:57 -0500 Subject: [PATCH] fix(format): refuse variable-length and reference VDS data from another file Their elements are global-heap IDs and object addresses in the source file. The VDS reader copied them raw, so anything decoding them against the virtual dataset's file got another object's data with no error. Same-file sources are unaffected. Found by the adversarial review. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/vds.rs | 25 ++++++++++++++++++ crates/clawhdf5/tests/vds_interop.rs | 38 ++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/crates/clawhdf5-format/src/vds.rs b/crates/clawhdf5-format/src/vds.rs index 23a3a52..f5d5237 100644 --- a/crates/clawhdf5-format/src/vds.rs +++ b/crates/clawhdf5-format/src/vds.rs @@ -831,6 +831,16 @@ impl<'a, 'r> Sources<'a, 'r> { path: &str, datatype: &Datatype, ) -> Result, FormatError> { + // Variable-length and reference elements are addresses into the file + // that holds them (global-heap IDs, object addresses). Copied out of + // another file they would be decoded against the virtual dataset's + // file and name some other object, so refuse rather than return them. + if file != "." && holds_file_addresses(datatype) { + return Err(vds_err(format!( + "VDS source {path} in {file}: variable-length and reference data \ + from another file is not supported" + ))); + } let Some(bytes) = self.file(file)? else { return Ok(None); }; @@ -841,6 +851,21 @@ impl<'a, 'r> Sources<'a, 'r> { } } +/// 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 { + match dt { + Datatype::VariableLength { .. } | Datatype::Reference { .. } => true, + Datatype::Compound { members, .. } => { + members.iter().any(|m| holds_file_addresses(&m.datatype)) + } + Datatype::Array { base_type, .. } | Datatype::Enumeration { base_type, .. } => { + holds_file_addresses(base_type) + } + _ => false, + } +} + /// An opened source dataset's object header. struct OpenSource { offset_size: u8, diff --git a/crates/clawhdf5/tests/vds_interop.rs b/crates/clawhdf5/tests/vds_interop.rs index 2346a5e..17069b0 100644 --- a/crates/clawhdf5/tests/vds_interop.rs +++ b/crates/clawhdf5/tests/vds_interop.rs @@ -290,6 +290,44 @@ expect("nested.h5", "v", "nested") assert_matches_libhdf5(dir.path(), "nested.h5", "v", "nested"); } +/// Variable-length and reference elements are addresses into their own file. +/// Copied raw from an external source they would be decoded against the +/// virtual dataset's file and name another object, so they are refused. +#[test] +fn vds_external_variable_length_source_is_an_error_not_foreign_addresses() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + generate( + dir.path(), + r#" +st = h5py.string_dtype() +with h5py.File("src.h5", "w") as s: + s.create_dataset("names", data=np.array(["alpha", "beta", "gamma"], dtype=object), dtype=st) + s.create_dataset("refs", data=[s.ref, s.ref], dtype=h5py.ref_dtype) +with h5py.File("v.h5", "w", libver="latest") as f: + f.create_dataset("pad", data=np.arange(64.0)) + lay = h5py.VirtualLayout(shape=(3,), dtype=st) + lay[:] = h5py.VirtualSource("src.h5", "names", shape=(3,)) + f.create_virtual_dataset("names", lay) + lay = h5py.VirtualLayout(shape=(2,), dtype=h5py.ref_dtype) + lay[:] = h5py.VirtualSource("src.h5", "refs", shape=(2,)) + f.create_virtual_dataset("refs", lay) +"#, + ); + let f = File::open(dir.path().join("v.h5")).unwrap(); + for name in ["names", "refs"] { + let err = f + .dataset(name) + .unwrap() + .read_selection(&clawhdf5_format::selection::Selection::All) + .expect_err("raw addresses from another file must not be returned"); + assert!( + err.to_string().contains("from another file"), + "{name}: unexpected error: {err}" + ); + } +} + // --------------------------------------------------------------------------- // Unlimited and printf-style mappings // ---------------------------------------------------------------------------