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) <[email protected]>

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}")
             }

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;
             }

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<String, FormatError> {
         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<u8> {
+        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);