From 90e050944f1ab7d6335e031f665af866b632ee53 Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 22:07:27 -0500 Subject: [PATCH] fix(format): refuse a local heap whose free list leaves the heap libhdf5 walks a local heap's free list when it loads the heap's data and refuses the heap ("bad heap free list") when a free block starts or ends outside the data segment, or links to offset 0. We never looked at the free list, so a damaged old-style group listed names read from the broken heap: once the user block of cve-2021-36977.h5 was applied, its root listed eight garbage names where libhdf5 fails. LocalHeap::validate_free_list (new) mirrors H5HL__fl_deserialize, with a cycle bound, and accepts H5HL_FREE_NULL (1) or an all-ones head as the end of the list. Like libhdf5 it runs when the first name is needed, not on parse, so an empty group with a damaged heap still lists as empty (cve-2018-13871.h5, cve-2024-29166.h5, gh-4431-poc-03.h5 keep matching h5py). Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 7 ++ crates/clawhdf5-format/src/error.rs | 6 + crates/clawhdf5-format/src/group_v1.rs | 12 ++ crates/clawhdf5-format/src/local_heap.rs | 99 ++++++++++++++- crates/clawhdf5/tests/local_heap_interop.rs | 131 ++++++++++++++++++++ docs/known-issues.md | 5 + 6 files changed, 258 insertions(+), 2 deletions(-) create mode 100644 crates/clawhdf5/tests/local_heap_interop.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a5283b..a4a5fa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -273,6 +273,13 @@ - CI keeps zlib-ng building and tested; the arm64 job no longer needs cmake. ### Correctness +- `clawhdf5-format` reader: an old-style group whose local heap has a free + list pointing outside the heap was listed with names read from the broken + heap (garbage names on `cve-2021-36977.h5` once its user block was + applied). libhdf5 refuses such a heap ("bad heap free list"); so do we now, + with `FormatError::InvalidLocalHeapFreeList`. As in libhdf5 the free list + is checked when the first name is read (`LocalHeap::validate_free_list`, + new), so an empty group with a damaged heap still lists as empty. - **Files with a user block** (`h5py.File(..., userblock_size=N)`, `h5jam`; the superblock at 512, 1024, …) could not be read: every address in the file is relative to the superblock, but it was applied from byte 0 diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index a54ca72..bb81939 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -80,6 +80,9 @@ pub enum FormatError { InvalidLocalHeapSignature, /// Invalid local heap version. InvalidLocalHeapVersion(u8), + /// A local heap's free list points outside its data segment (libhdf5: + /// "bad heap free list"). + InvalidLocalHeapFreeList, /// Invalid B-tree v1 signature. InvalidBTreeSignature, /// Invalid B-tree node type. @@ -278,6 +281,9 @@ impl fmt::Display for FormatError { FormatError::InvalidLocalHeapSignature => { write!(f, "invalid local heap signature") } + FormatError::InvalidLocalHeapFreeList => { + write!(f, "bad local heap free list") + } FormatError::InvalidLocalHeapVersion(v) => { write!(f, "invalid local heap version: {v}") } diff --git a/crates/clawhdf5-format/src/group_v1.rs b/crates/clawhdf5-format/src/group_v1.rs index 989f826..e55ad16 100644 --- a/crates/clawhdf5-format/src/group_v1.rs +++ b/crates/clawhdf5-format/src/group_v1.rs @@ -45,9 +45,16 @@ pub fn resolve_v1_group_entries( )?; let mut entries = Vec::new(); + let mut heap_checked = false; for snod_addr in snod_addrs { let snod = SymbolTableNode::parse(file_data, snod_addr as usize, offset_size)?; for entry in &snod.entries { + // Like libhdf5, look at the heap's free list only once a name is + // needed: an empty group with a damaged heap still lists. + if !heap_checked { + heap.validate_free_list(file_data, length_size)?; + heap_checked = true; + } let name = heap.read_string(file_data, entry.link_name_offset)?; entries.push(GroupEntry { name, @@ -85,12 +92,17 @@ pub fn find_v1_soft_link( offset_size, length_size, )?; + let mut heap_checked = false; for snod_addr in snod_addrs { let snod = SymbolTableNode::parse(file_data, snod_addr as usize, offset_size)?; for entry in &snod.entries { if entry.cache_type != CACHE_TYPE_SOFT_LINK { continue; } + if !heap_checked { + heap.validate_free_list(file_data, length_size)?; + heap_checked = true; + } if heap.read_string(file_data, entry.link_name_offset)? != name { continue; } diff --git a/crates/clawhdf5-format/src/local_heap.rs b/crates/clawhdf5-format/src/local_heap.rs index 33e3433..39e9b26 100644 --- a/crates/clawhdf5-format/src/local_heap.rs +++ b/crates/clawhdf5-format/src/local_heap.rs @@ -87,6 +87,57 @@ impl LocalHeap { }) } + /// Walk the free list the way libhdf5 does when it loads a heap's data + /// (`H5HL__fl_deserialize`), rejecting a heap whose free list points + /// outside the data segment. libhdf5 refuses such a heap ("bad heap free + /// list"), and names read from it would be garbage. + /// + /// libhdf5 only loads a heap when it needs a name from it (an empty + /// group's broken heap goes unnoticed), so call this before the first + /// [`Self::read_string`], not on parse. + /// + /// The end of the list is `H5HL_FREE_NULL` (1); an all-ones value (the + /// undefined address) is accepted as "no free list" too. + pub fn validate_free_list(&self, file_data: &[u8], length_size: u8) -> Result<(), FormatError> { + const FREE_NULL: u64 = 1; + let ls = length_size as usize; + let undefined = if ls >= 8 { + u64::MAX + } else { + (1u64 << (8 * ls)) - 1 + }; + let size = self.data_segment_size; + let seg = self.data_segment_address; + let mut next = self.free_list_head_offset; + // Each free block holds two lengths, so a list longer than this + // revisits a block: a cycle. + let max_blocks = size / (2 * ls as u64) + 1; + let mut walked = 0u64; + while next != FREE_NULL && next != undefined { + if next >= size || walked >= max_blocks { + return Err(FormatError::InvalidLocalHeapFreeList); + } + walked += 1; + let at = seg + .checked_add(next) + .and_then(|a| usize::try_from(a).ok()) + .ok_or(FormatError::InvalidLocalHeapFreeList)?; + let block_offset = next; + next = read_offset(file_data, at, length_size)?; + if next == 0 { + return Err(FormatError::InvalidLocalHeapFreeList); + } + let block_size = read_offset(file_data, at + ls, length_size)?; + if block_offset + .checked_add(block_size) + .is_none_or(|end| end > size) + { + return Err(FormatError::InvalidLocalHeapFreeList); + } + } + Ok(()) + } + /// Read a null-terminated string from the heap's data segment at the given byte offset. pub fn read_string(&self, file_data: &[u8], string_offset: u64) -> Result { let seg_addr = self.data_segment_address as usize; @@ -162,8 +213,8 @@ mod tests { // data_segment_size write_val(&mut file, pos, data_seg_size as u64, length_size); pos += length_size as usize; - // free_list_head_offset - write_val(&mut file, pos, 0xFFFFFFFF, length_size); + // free_list_head_offset: H5HL_FREE_NULL (no free space) + write_val(&mut file, pos, 1, length_size); pos += length_size as usize; // data_segment_address write_val(&mut file, pos, data_seg_offset as u64, offset_size); @@ -243,6 +294,50 @@ mod tests { assert_eq!(s, "test"); } + /// Heap with data segment `[a, b, c, 0-padding]` whose free list starts + /// at `head` and has one block `(next, size)` at offset 8. + fn heap_with_free_block(head: u64, next: u64, size: u64) -> Vec { + let mut file = build_heap_file(0, 100, &["abcdefg"], 8, 8); + file.resize(200, 0); + write_val(&mut file, 8, 32, 8); // data segment size + write_val(&mut file, 16, head, 8); + write_val(&mut file, 108, next, 8); + write_val(&mut file, 116, size, 8); + file + } + + #[test] + fn free_list_inside_the_segment_is_accepted() { + let file = heap_with_free_block(8, 1, 24); + let heap = LocalHeap::parse(&file, 0, 8, 8).unwrap(); + heap.validate_free_list(&file, 8).unwrap(); + assert_eq!(heap.read_string(&file, 0).unwrap(), "abcdefg"); + // An all-ones head is "no free list" too. + let file = heap_with_free_block(u64::MAX, 0, 0); + let heap = LocalHeap::parse(&file, 0, 8, 8).unwrap(); + assert!(heap.validate_free_list(&file, 8).is_ok()); + } + + #[test] + fn bad_free_list_is_rejected_like_libhdf5() { + for (head, next, size, why) in [ + (40, 1, 8, "head past the segment"), + (8, 1, 25, "block runs past the segment"), + (8, 0, 8, "next offset of zero"), + (8, 8, 8, "cycle"), + (8, 999, 8, "next past the segment"), + ] { + let file = heap_with_free_block(head, next, size); + // The header itself parses; the free list is checked on use. + let heap = LocalHeap::parse(&file, 0, 8, 8).unwrap(); + assert_eq!( + heap.validate_free_list(&file, 8).unwrap_err(), + FormatError::InvalidLocalHeapFreeList, + "{why}" + ); + } + } + #[test] fn invalid_version() { let mut file = build_heap_file(0, 100, &["x"], 8, 8); diff --git a/crates/clawhdf5/tests/local_heap_interop.rs b/crates/clawhdf5/tests/local_heap_interop.rs new file mode 100644 index 0000000..02aa6e0 --- /dev/null +++ b/crates/clawhdf5/tests/local_heap_interop.rs @@ -0,0 +1,131 @@ +//! Old-style (symbol-table) groups keep link names in a local heap. libhdf5 +//! validates the heap's free list when it loads the heap and refuses the +//! group ("bad heap free list") when the list points outside the heap; we +//! must refuse too instead of listing names read from a broken heap. Like +//! libhdf5, the check happens when a name is needed, so an empty group with +//! a broken heap still lists. +//! +//! h5py writes the files; skipped when python3 with h5py is unavailable, +//! unless `CLAWHDF5_REQUIRE_INTEROP=1`. + +use std::process::Command; + +use clawhdf5::File; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn interop_required() -> bool { + std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1") +} + +fn python_available() -> bool { + Command::new(python()) + .args(["-c", "import h5py, numpy"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +macro_rules! skip_if_no_python { + () => { + if !python_available() { + assert!( + !interop_required(), + "CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available" + ); + eprintln!("SKIP: python3 with h5py not available"); + return; + } + }; +} + +fn run_python(script: &str) -> String { + let output = Command::new(python()) + .args(["-c", script]) + .output() + .expect("failed to run python"); + assert!( + output.status.success(), + "python failed:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).into_owned() +} + +#[test] +fn local_heap_free_list_checked_like_libhdf5() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let good = dir.path().join("good.h5"); + // Writes `good.h5` (a deleted link leaves a real free block in the root + // group's heap) and two copies whose root heap free list is broken; for + // each prints what h5py lists, or `ERROR`. + let script = format!( + r#" +import h5py, struct +good = "{good}" +with h5py.File(good, "w", libver="earliest") as f: + for name in ("alpha", "beta", "gamma"): + f.create_group(name) + del f["beta"] +data = bytearray(open(good, "rb").read()) +heap = data.find(b"HEAP") # the root group's heap is written first +size, head, seg = struct.unpack_from(" = out.lines().collect(); + assert_eq!( + lines, + [ + "good alpha gamma", + "bad_head ERROR", + "bad_block ERROR", + "bad_empty" + ], + "h5py's view changed" + ); + + let file = File::open(&good).unwrap(); + let mut groups = file.root().groups().unwrap(); + groups.sort(); + assert_eq!(groups, ["alpha", "gamma"]); + + for name in ["bad_head", "bad_block"] { + let file = File::open(dir.path().join(format!("{name}.h5"))).unwrap(); + let listed = file.root().groups(); + assert!( + listed.is_err(), + "{name}: listed {listed:?} from a heap libhdf5 rejects" + ); + } + + let file = File::open(dir.path().join("bad_empty.h5")).unwrap(); + assert_eq!(file.root().groups().unwrap(), Vec::::new()); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index 6f84c6a..4d57750 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -86,6 +86,11 @@ the VDS item, which is marked. - Groups with a user-defined link type (e.g. 187) cannot be listed. - Dense groups with more than about 22 000 links cannot be listed. - Soft links are left out of `datasets()`. + - **Wrong data (found while fixing user blocks):** an old-style group whose + local-heap free list points outside the heap listed garbage names where + libhdf5 refuses the heap. **Fixed 2026-09-25** + (`InvalidLocalHeapFreeList`, checked when a name is first read, as + libhdf5 does). - **Dense attributes:** a large attribute stored as a fractal-heap "huge" object makes every attribute on the object fail. This affects real NetCDF files (`issue671.nc`).