format: verify B-tree v2 internal node checksums
Only leaves and the header were checked. Harmless while every lookup read the whole tree, but the indexed lookup prunes children by the keys stored in internal nodes, so one corrupted byte there could route a name to the wrong child and report it missing with no error. A BTIN whose lookup3 checksum does not match is now ChecksumMismatch on every read (lookups and full traversals), as in libhdf5. Test: one byte of the root BTIN of the 35 001-link h5py group's name index changed -> lookups, paths and listings through File, MmapFile and LazyFile all fail with ChecksumMismatch, and h5py refuses both. Before, lookups returned Ok. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -355,6 +355,23 @@ fn read_internal_node(
|
||||
pos += total_nrec_width; // skip total records in subtree
|
||||
children.push((addr, child_nrec));
|
||||
}
|
||||
|
||||
// The checksum follows the child pointers and covers the node up to it.
|
||||
// Lookups prune children by the keys in this node, so an unverified
|
||||
// internal node could hide a record without any error: libhdf5 refuses
|
||||
// a mismatch here, and so does this.
|
||||
#[cfg(feature = "checksum")]
|
||||
{
|
||||
ensure_len(file_data, pos, 4)?;
|
||||
let stored = LittleEndian::read_u32(&file_data[pos..pos + 4]);
|
||||
let computed = crate::checksum::jenkins_lookup3(&file_data[offset..pos]);
|
||||
if computed != stored {
|
||||
return Err(FormatError::ChecksumMismatch {
|
||||
expected: stored,
|
||||
computed,
|
||||
});
|
||||
}
|
||||
}
|
||||
Ok((records_start, children))
|
||||
}
|
||||
|
||||
@@ -733,6 +750,8 @@ mod tests {
|
||||
buf.extend_from_slice(&child_nrec.to_le_bytes()[..nrec_width]);
|
||||
buf.resize(buf.len() + total_width, 0);
|
||||
}
|
||||
let sum = crate::checksum::jenkins_lookup3(&buf);
|
||||
buf.extend_from_slice(&sum.to_le_bytes());
|
||||
buf
|
||||
}
|
||||
|
||||
|
||||
@@ -411,3 +411,87 @@ fn links_of_every_kind_resolve_by_name_as_in_h5py() {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The link name index (v2 B-tree, record type 5) of the big group: its
|
||||
/// depth and root node address, read from the one type-5 `BTHD` in the file.
|
||||
fn name_index_root(bytes: &[u8]) -> (u16, usize) {
|
||||
let headers: Vec<usize> = bytes
|
||||
.windows(4)
|
||||
.enumerate()
|
||||
.filter(|(i, w)| *w == b"BTHD" && bytes.get(i + 5) == Some(&5))
|
||||
.map(|(i, _)| i)
|
||||
.collect();
|
||||
assert_eq!(headers.len(), 1, "type-5 B-tree headers at {headers:?}");
|
||||
let h = headers[0];
|
||||
// signature, version, type, node size (4), record size (2), depth (2),
|
||||
// split and merge percent, root address (8).
|
||||
let depth = u16::from_le_bytes([bytes[h + 12], bytes[h + 13]]);
|
||||
let root = u64::from_le_bytes(bytes[h + 16..h + 24].try_into().unwrap());
|
||||
(depth, usize::try_from(root).unwrap())
|
||||
}
|
||||
|
||||
/// One byte changed in a key of the name index's root (an internal node)
|
||||
/// must be an error, not a name quietly routed to the wrong child and
|
||||
/// reported missing: lookups prune children by those keys. libhdf5 checks
|
||||
/// the internal node's checksum and refuses the group; so must we, for a
|
||||
/// lookup and for a listing.
|
||||
#[test]
|
||||
fn a_corrupt_internal_index_node_is_an_error_not_a_missing_name() {
|
||||
skip_if_no_python!();
|
||||
let fx = fixture();
|
||||
let mut bytes = std::fs::read(&fx.path).unwrap();
|
||||
let (depth, root) = name_index_root(&bytes);
|
||||
assert!(depth >= 2, "want a deep index, got depth {depth}");
|
||||
assert_eq!(&bytes[root..root + 4], b"BTIN");
|
||||
// Signature, version, type, then record 0: its name hash comes first.
|
||||
bytes[root + 6] ^= 0x5a;
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let bad = dir.path().join("bad.h5");
|
||||
std::fs::write(&bad, &bytes).unwrap();
|
||||
let bad = bad.display().to_string();
|
||||
|
||||
let is_checksum = |e: &clawhdf5::Error| {
|
||||
matches!(
|
||||
e,
|
||||
clawhdf5::Error::Format(FormatError::ChecksumMismatch { .. })
|
||||
)
|
||||
};
|
||||
let f = File::open(&bad).unwrap();
|
||||
let g = f.group("g").unwrap();
|
||||
// Every name, present or not, goes through the root.
|
||||
for name in fx.links.keys().step_by(97).chain(&fx.missing_links) {
|
||||
let err = g.dataset(name).map(|_| ()).unwrap_err();
|
||||
assert!(is_checksum(&err), "dataset({name:?}): {err:?}");
|
||||
}
|
||||
let err = f.dataset("/g/n0").map(|_| ()).unwrap_err();
|
||||
assert!(is_checksum(&err), "path: {err:?}");
|
||||
let err = g.datasets().unwrap_err();
|
||||
assert!(is_checksum(&err), "listing: {err:?}");
|
||||
let err = g.entries().unwrap_err();
|
||||
assert!(is_checksum(&err), "entries: {err:?}");
|
||||
|
||||
let m = MmapFile::open(&bad).unwrap();
|
||||
let mg = m.group("g").unwrap();
|
||||
assert!(mg.dataset("n0").is_err_and(|e| is_checksum(&e)));
|
||||
assert!(mg.datasets().is_err_and(|e| is_checksum(&e)));
|
||||
let l = LazyFile::open_mmap(&bad).unwrap();
|
||||
let lg = l.group("g").unwrap();
|
||||
assert!(lg.dataset("n0").is_err_and(|e| is_checksum(&e)));
|
||||
assert!(lg.datasets().is_err_and(|e| is_checksum(&e)));
|
||||
|
||||
// libhdf5 refuses both too.
|
||||
let out = run_python(&format!(
|
||||
"import h5py\n\
|
||||
r = []\n\
|
||||
with h5py.File(r'{bad}', 'r') as f:\n\
|
||||
\x20 g = f['g']\n\
|
||||
\x20 for op in (lambda: g['n0'], lambda: list(g)):\n\
|
||||
\x20 try:\n\
|
||||
\x20 op()\n\
|
||||
\x20 r.append('ok')\n\
|
||||
\x20 except Exception as e:\n\
|
||||
\x20 r.append('checksum' if 'checksum' in str(e) else repr(e))\n\
|
||||
print(' '.join(r))",
|
||||
));
|
||||
assert_eq!(out, "checksum checksum");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user