fix(tools): h5rs check --data follows VL data into the global heap

The README said check skips only "global heap collections other than
those a value read touches", but read_dataset returns the raw heap IDs,
so no collection was ever read: a file whose global heap collection
claims a 4 GiB object passed `check --data` with no problems, while
h5dump (and h5rs dump/diff) fail on it.

With --data, every variable-length element (strings and sequences, also
inside compounds, arrays and nested sequences) of every dataset and
attribute is followed into its collection. A collection that does not
parse, a missing heap object, or a sequence longer than its heap object
is a problem at the collection's address, once per object; the summary
counts the collections read.

Measured on tank, 2026-09-26: the 418 fully-read conformance ok files
still pass (scripts/h5rs-check-ok-files.sh --data, 0 flagged), and
`check --data` now flags 152 of the 180 CVE-corpus files (was 147); of
the 28 it passes, h5dump 1.14.6 rejects 21 (was 26 of 33).

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

crates/clawhdf5-tools/README.md

@@ -216,7 +216,12 @@ extension) and checks:
   no two pieces overlap.
 
 `--data` also reads every dataset, decoding every chunk through its filters
-(which catches corrupt compressed data and Fletcher-32 mismatches). Data the
+(which catches corrupt compressed data and Fletcher-32 mismatches), and
+follows every variable-length element (strings and sequences, also inside
+compounds and arrays) of every dataset and attribute into its global heap
+collection: a collection that does not parse, a missing heap object, or a
+sequence longer than its heap object is a problem at the collection's
+address. Data the
 tool cannot decode (a filter it does not implement, such as szip, or a
 dataset over `--max-bytes`) is a `note:`, not a problem. Every problem is
 printed with the address of the structure involved; the exit status is 0
@@ -228,11 +233,11 @@ also covers the structures HDF5 1.10+ writes (fixed/extensible array and v2
 B-tree chunk indexes, and version 3 superblocks).
 
 What it does not check: free-space manager and shared-message (SOHM) table
-checksums, global heap collections other than those a value read touches,
-and objects reachable only by external links. It validates with
+checksums, global heap collections no variable-length value points into (and
+none at all without `--data`), and objects reachable only by external links. It validates with
 clawhdf5's parsers, so it accepts what they accept: some header damage that
 libhdf5 refuses goes unreported (of the 180 files of the HDF Group's CVE
-corpus, `check --data` passes 33, and h5dump 1.14.6 rejects 26 of those;
+corpus, `check --data` passes 28, and h5dump 1.14.6 rejects 21 of those;
 tank, 2026-09-26).
 
 ## Robustness

crates/clawhdf5-tools/src/check.rs

@@ -7,7 +7,7 @@
 //! data lies inside the file without overlapping other raw data. Every
 //! problem is reported with the address of the structure involved.
 
-use std::collections::HashSet;
+use std::collections::{BTreeMap, HashSet};
 use std::panic::{self, AssertUnwindSafe};
 
 use clawhdf5_format::attribute_info::AttributeInfoMessage;
@@ -39,7 +39,8 @@ problem is printed with the address of the structure involved.
 
   --data          also read every dataset, decoding every chunk through its
                   filters (catches corrupt compressed data and Fletcher-32
-                  mismatches)
+                  mismatches), and follow every variable-length element of
+                  datasets and attributes into its global heap collection
   -q, --quiet     print only the problems, not the summary
   --max-bytes N   largest dataset read by --data (default 1 GiB)
 
@@ -48,6 +49,21 @@ found, 3 internal error.";
 
 const MAX_CHUNKS_CHECKED: usize = 10_000_000;
 
+/// Whether values of `dt` hold variable-length data (in the global heap).
+fn has_vl(dt: &Datatype, depth: u32) -> bool {
+    if depth > 32 {
+        return false;
+    }
+    match dt {
+        Datatype::VariableLength { .. } => true,
+        Datatype::Compound { members, .. } => {
+            members.iter().any(|m| has_vl(&m.datatype, depth + 1))
+        }
+        Datatype::Array { base_type, .. } => has_vl(base_type, depth + 1),
+        _ => false,
+    }
+}
+
 #[derive(Default)]
 struct Counts {
     objects: u64,
@@ -57,6 +73,9 @@ struct Counts {
     messages: u64,
     chunks: u64,
     datasets_read: u64,
+    /// Global heap collections that variable-length data points into,
+    /// parsed without error (with --data).
+    global_heaps: u64,
     sb_checksum: u64,
     ohdr_v2: u64,
     btree_v2: u64,
@@ -83,6 +102,8 @@ struct Checker<'a> {
     extents: Vec<(u64, u64, String)>,
     heaps_seen: HashSet<u64>,
     btrees_seen: HashSet<u64>,
+    /// Global heap collections already read (with --data).
+    gcols_seen: HashSet<u64>,
     panicked: bool,
 }
 
@@ -135,6 +156,7 @@ pub fn run(args: &mut Args, out: &mut Out) -> std::io::Result<i32> {
         extents: Vec::new(),
         heaps_seen: HashSet::new(),
         btrees_seen: HashSet::new(),
+        gcols_seen: HashSet::new(),
         panicked: false,
     };
     c.superblock();
@@ -318,10 +340,16 @@ impl Checker<'_> {
         }
         self.messages(addr, path, h);
         match self.h5.attributes(h) {
-            Ok((_, errs)) => {
+            Ok((attrs, errs)) => {
                 for e in errs {
                     self.problem(addr, path, format!("attribute: {e}"));
                 }
+                if self.read_data {
+                    for a in &attrs {
+                        let what = format!("attribute \"{}\": ", a.name);
+                        self.vl_data(addr, path, &what, &a.datatype, &a.dataspace, &a.raw_data);
+                    }
+                }
             }
             Err(e) => self.err(addr, path, &e),
         }
@@ -598,7 +626,10 @@ impl Checker<'_> {
         }
         if self.read_data {
             match self.h5.read_dataset(path, dt, ds) {
-                Ok(_) => self.counts.datasets_read += 1,
+                Ok(raw) => {
+                    self.counts.datasets_read += 1;
+                    self.vl_data(addr, path, "", dt, ds, &raw);
+                }
                 // Valid data this tool cannot decode is not a problem with
                 // the file.
                 Err(e) if e.kind != ErrorKind::Corrupt => self.notes.push(Problem {
@@ -611,6 +642,141 @@ impl Checker<'_> {
         }
     }
 
+    /// With --data: follow every variable-length element of `raw` (the
+    /// values of a dataset or attribute) into the global heap, so a damaged
+    /// collection, a missing heap object or a sequence longer than its heap
+    /// object is reported at the collection's address. Each bad collection
+    /// is reported once per object.
+    fn vl_data(
+        &mut self,
+        addr: u64,
+        path: &str,
+        what: &str,
+        dt: &Datatype,
+        ds: &Dataspace,
+        raw: &[u8],
+    ) {
+        if !has_vl(dt, 0) {
+            return;
+        }
+        let n = crate::h5::num_elements(ds).unwrap_or(0);
+        let size = dt.type_size() as usize;
+        let mut bad: BTreeMap<u64, String> = BTreeMap::new();
+        for i in 0..n {
+            let Some(b) = usize::try_from(i)
+                .ok()
+                .and_then(|i| i.checked_mul(size))
+                .and_then(|s| raw.get(s..s.checked_add(size)?))
+            else {
+                self.problem(
+                    addr,
+                    path,
+                    format!("{what}element {i} is past the data read"),
+                );
+                break;
+            };
+            self.vl_element(dt, b, 0, &mut bad);
+            if bad.len() >= 100 {
+                break;
+            }
+        }
+        for (a, msg) in bad {
+            self.problem(a, path, format!("{what}variable-length data: {msg}"));
+        }
+    }
+
+    fn vl_element(&mut self, dt: &Datatype, b: &[u8], depth: u32, bad: &mut BTreeMap<u64, String>) {
+        if depth > 32 {
+            return;
+        }
+        match dt {
+            Datatype::VariableLength {
+                is_string,
+                base_type,
+                ..
+            } => {
+                let os = usize::from(self.h5.os());
+                let (Some(lenb), Some(addrb), Some(idxb)) =
+                    (b.get(..4), b.get(4..4 + os), b.get(4 + os..8 + os))
+                else {
+                    return;
+                };
+                let le = |x: &[u8]| {
+                    x.iter()
+                        .enumerate()
+                        .fold(0u64, |a, (i, &v)| a | (u64::from(v) << (8 * i)))
+                };
+                let (len, gcol, idx) = (le(lenb), le(addrb), le(idxb));
+                let undef = if os >= 8 {
+                    u64::MAX
+                } else {
+                    (1u64 << (8 * os)) - 1
+                };
+                if len == 0 || gcol == 0 || gcol == undef || bad.contains_key(&gcol) {
+                    return;
+                }
+                let obj = match self.h5.heap_object(gcol, idx as u32) {
+                    Ok(o) => o,
+                    Err(e) => {
+                        bad.insert(e.addr.unwrap_or(gcol), e.msg);
+                        return;
+                    }
+                };
+                if self.gcols_seen.insert(gcol) {
+                    self.counts.global_heaps += 1;
+                }
+                let bs = if *is_string {
+                    1
+                } else {
+                    u64::from(base_type.type_size())
+                };
+                if len
+                    .checked_mul(bs)
+                    .is_none_or(|need| need > obj.len() as u64)
+                {
+                    bad.insert(
+                        gcol,
+                        format!(
+                            "global heap object {idx} holds {} bytes; the element needs {len} x {bs}",
+                            obj.len()
+                        ),
+                    );
+                    return;
+                }
+                if !*is_string && bs > 0 && has_vl(base_type, depth + 1) {
+                    let bs = bs as usize;
+                    for k in 0..len as usize {
+                        self.vl_element(base_type, &obj[k * bs..(k + 1) * bs], depth + 1, bad);
+                    }
+                }
+            }
+            Datatype::Compound { members, .. } => {
+                for m in members {
+                    if let Some(mb) = usize::try_from(m.byte_offset).ok().and_then(|o| b.get(o..)) {
+                        self.vl_element(&m.datatype, mb, depth + 1, bad);
+                    }
+                }
+            }
+            Datatype::Array {
+                base_type,
+                dimensions,
+            } => {
+                let bs = base_type.type_size() as usize;
+                let n = dimensions
+                    .iter()
+                    .try_fold(1usize, |a, &d| a.checked_mul(d as usize))
+                    .unwrap_or(usize::MAX);
+                for k in 0..n {
+                    match b.get(k * bs..(k + 1) * bs) {
+                        Some(eb) => self.vl_element(base_type, eb, depth + 1, bad),
+                        None => break,
+                    }
+                }
+            }
+            _ => {}
+        }
+    }
+
     fn chunked(
         &mut self,
         addr: u64,
@@ -814,7 +980,11 @@ impl Checker<'_> {
             c.chunk_index_checksummed
         )?;
         if self.read_data {
-            writeln!(out.o, "datasets read: {}", c.datasets_read)?;
+            writeln!(
+                out.o,
+                "datasets read: {}, global heap collections read: {}",
+                c.datasets_read, c.global_heaps
+            )?;
         }
         match self.problems.len() {
             0 => writeln!(out.o, "no problems found"),

crates/clawhdf5-tools/tests/h5rs_interop.rs

@@ -639,6 +639,44 @@ fn check_flags_every_corrupted_checksum() {
     }
 }
 
+/// `check --data` follows variable-length elements into the global heap:
+/// a damaged collection is reported at its address, for the dataset and
+/// the attribute that point into it. Without --data it is not read.
+#[test]
+fn check_data_follows_vl_data_into_the_global_heap() {
+    let Some(f) = generate() else { return };
+    for name in LIBVERS {
+        let mut data = std::fs::read(f.path(name)).unwrap();
+        let gcols: Vec<usize> = (0..data.len() - 4)
+            .filter(|&i| &data[i..i + 4] == b"GCOL")
+            .collect();
+        assert!(!gcols.is_empty(), "{name}: no global heap");
+        // "GCOL" version(1) reserved(3) size(8), then heap object 1:
+        // index(2) refcount(2) reserved(4) size(8) — claim 4 GiB.
+        let at = gcols[0];
+        data[at + 24..at + 28].fill(0xff);
+        let bad = f.path(&format!("bad-gcol-{name}"));
+        std::fs::write(&bad, &data).unwrap();
+        let bad = bad.to_string_lossy().into_owned();
+        let o = h5rs(&["check", "--data", &bad]);
+        let s = stdout(&o);
+        assert_eq!(code(&o), 1, "{name}: {s}");
+        let want = format!("problem: {at:#x} /vlstr: variable-length data: global heap");
+        assert!(s.contains(&want), "{name}: no {want:?} in\n{s}");
+        // h5dump refuses the file too.
+        if tool_available("h5dump") {
+            assert_ne!(code(&run("h5dump", &[&bad])), 0, "{name}: h5dump read it");
+        }
+        assert_eq!(code(&h5rs(&["check", &bad])), 0, "{name}: without --data");
+        let ok = h5rs(&["check", "--data", &f.p(name)]);
+        assert!(
+            stdout(&ok).contains(&format!("global heap collections read: {}", gcols.len())),
+            "{name}: {}",
+            stdout(&ok)
+        );
+    }
+}
+
 #[test]
 fn check_flags_a_truncated_file() {
     let Some(f) = generate() else { return };