quantumclaw/clawhdf5
3
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(format): verify Fixed and Extensible Array checksums

Browse Source 
Every structure in both chunk indexes — header, index block, super
block, data block and each data block page — carries a Jenkins lookup3
checksum, and all of them were parsed past and ignored.

What that costs is not a warning but correct data. Flip one low bit of a
chunk address and the index still has the right shape, the address still
lands inside the file, and the reader returns whatever bytes now sit
there as that chunk's contents. Nothing else in the parse can tell.

Verified in both directions. The checksums accept files written by
HDF5 2.0 from 100 to 200 000 chunks — dense, sparse, gzip-filtered and
paged — which also confirms the block layouts byte for byte, since a
wrong offset would fail every file. And an interop test corrupts an
address to check the read fails instead of returning data: removing the
verification makes that test fail with "corruption produced data instead
of an error", which is what it is there to prove.

The first version of that test passed with verification disabled — it
corrupted a byte a structural check already rejected, so it proved
nothing. Worth recording, since a test that passes for the wrong reason
looks exactly like coverage.

Hand-built fixtures now stamp real checksums, as HDF5 writers do, and
the Extensible Array ones no longer describe the superseded layout.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-20 17:41:41 -07:00
co-authored by Claude Opus 5
parent 0bc7a293ae
commit b41272487a
4 changed files with 349 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/clawhdf5-format/src/fixed_array.rs
+106 -2
View File
@@ -9,6 +9,31 @@ use alloc::{format, vec, vec::Vec};
use crate::chunked_read::ChunkInfo;
use crate::error::FormatError;
/// Verify the Jenkins lookup3 checksum stored immediately after
/// `data[start..end]`, as every Fixed Array structure carries one.
///
/// A corrupt chunk index silently yields addresses pointing at the wrong
/// bytes, so a mismatch has to be an error rather than a shrug: without this
/// the damage surfaces as plausible-looking data from the wrong chunk.
#[cfg(feature = "checksum")]
fn verify_checksum(data: &[u8], start: usize, end: usize) -> Result<(), FormatError> {
ensure_len(data, end, 4)?;
let stored = u32::from_le_bytes([data[end], data[end + 1], data[end + 2], data[end + 3]]);
let computed = crate::checksum::jenkins_lookup3(&data[start..end]);
if computed != stored {
return Err(FormatError::ChecksumMismatch {
expected: stored,
computed,
});
}
Ok(())
}
#[cfg(not(feature = "checksum"))]
fn verify_checksum(_data: &[u8], _start: usize, _end: usize) -> Result<(), FormatError> {
Ok(())
}
/// Parsed Fixed Array header (FAHD).
#[derive(Debug, Clone)]
pub struct FixedArrayHeader {
@@ -103,6 +128,8 @@ impl FixedArrayHeader {
let num_elements = read_length(d, pos, length_size)?;
pos += length_size as usize;
let data_block_address = read_offset(d, pos, offset_size)?;
pos += offset_size as usize;
verify_checksum(file_data, offset, offset + pos)?;
Ok(FixedArrayHeader {
client_id,
@@ -223,7 +250,8 @@ pub fn read_fixed_array_chunks(
if !is_paged {
// Non-paged: prefix, then `num_elements` elements packed directly,
// then a trailing checksum (which we don't validate).
// then a checksum over both.
verify_checksum(file_data, db_offset, elem_at(elements_start, num_elements)?)?;
for i in 0..num_elements {
push_element(i, elem_at(elements_start, i)?, &mut chunks)?;
}
@@ -254,6 +282,9 @@ pub fn read_fixed_array_chunks(
available: file_data.len(),
});
}
// The prefix and page bitmap are covered by their own checksum, and each
// initialised page by one of its own.
verify_checksum(file_data, db_offset, bitmap_start + bitmap_size)?;
for p in 0..npages {
let page_first = p * page_nelmts; // < num_elements, cannot overflow
@@ -270,6 +301,7 @@ pub fn read_fixed_array_chunks(
.checked_mul(page_stride)
.and_then(|o| pages_start.checked_add(o))
.ok_or_else(stride_overflow)?;
verify_checksum(file_data, page_off, elem_at(page_off, page_count)?)?;
for e in 0..page_count {
push_element(page_first + e, elem_at(page_off, e)?, &mut chunks)?;
}
@@ -374,6 +406,14 @@ fn read_variable_length(data: &[u8], size: usize) -> Result<u64, FormatError> {
mod tests {
use super::*;
/// Stamp the Jenkins checksum a real file would carry over
/// `data[start..end]`, writing it at `end`. Fixtures built by hand need
/// this now that the reader validates it — as every HDF5 writer does.
fn stamp_checksum(data: &mut [u8], start: usize, end: usize) {
let sum = crate::checksum::jenkins_lookup3(&data[start..end]);
data[end..end + 4].copy_from_slice(&sum.to_le_bytes());
}
#[test]
fn index_to_offsets_1d() {
let num_chunks = vec![5u64];
@@ -439,7 +479,7 @@ mod tests {
buf[8..16].copy_from_slice(&5u64.to_le_bytes());
// data_block_address (offset_size=8)
buf[16..24].copy_from_slice(&0x1000u64.to_le_bytes());
// checksum (4 bytes, we don't validate in parse)
stamp_checksum(&mut buf, 0, 24);
let header = FixedArrayHeader::parse(&buf, 0, 8, 8).unwrap();
assert_eq!(header.client_id, 1);
@@ -449,6 +489,54 @@ mod tests {
assert_eq!(header.data_block_address, 0x1000);
}
/// Corruption anywhere in the index must be an error, not a wrong
/// address. Every structure carries a checksum; flipping a bit in each in
/// turn must be caught, because the alternative is reading a chunk from
/// the wrong offset and returning it as data.
#[test]
fn corrupting_any_fixed_array_structure_is_detected() {
let build = || -> (Vec<u8>, usize) {
let (os, fahd, db) = (8usize, 0x100usize, 0x200usize);
let mut f = vec![0u8; 0x3000];
f[fahd..fahd + 4].copy_from_slice(b"FAHD");
f[fahd + 6] = os as u8;
f[fahd + 7] = 10;
f[fahd + 8..fahd + 16].copy_from_slice(&3u64.to_le_bytes());
f[fahd + 16..fahd + 24].copy_from_slice(&(db as u64).to_le_bytes());
stamp_checksum(&mut f, fahd, fahd + 24);
f[db..db + 4].copy_from_slice(b"FADB");
f[db + 6..db + 14].copy_from_slice(&(fahd as u64).to_le_bytes());
let elems = db + 6 + os;
for i in 0..3usize {
let addr = 0x1000u64 + i as u64 * 0x100;
f[elems + i * os..elems + (i + 1) * os].copy_from_slice(&addr.to_le_bytes());
}
stamp_checksum(&mut f, db, elems + 3 * os);
(f, fahd)
};
let read = |f: &[u8], fahd: usize| -> Result<Vec<ChunkInfo>, FormatError> {
let h = FixedArrayHeader::parse(f, fahd, 8, 8)?;
read_fixed_array_chunks(f, &h, &[60], &[20], 8, 8, 8)
};
let (clean, fahd) = build();
assert!(read(&clean, fahd).is_ok(), "the intact fixture must read");
// A byte inside the header, and one inside a data block element.
for &at in &[0x108usize, 0x210usize] {
let (mut damaged, fahd) = build();
damaged[at] ^= 0x01;
assert!(
matches!(
read(&damaged, fahd),
Err(FormatError::ChecksumMismatch { .. })
),
"corruption at {at:#x} went undetected"
);
}
}
#[test]
fn parse_fixed_array_header_invalid_signature() {
let mut buf = vec![0u8; 256];
@@ -469,6 +557,7 @@ mod tests {
buf[fahd + 7] = 200; // max_nelmts_bits — absurd, would overflow a shift
buf[fahd + 8..fahd + 16].copy_from_slice(&3u64.to_le_bytes()); // num_elements
buf[fahd + 16..fahd + 24].copy_from_slice(&0x100u64.to_le_bytes());
stamp_checksum(&mut buf, fahd, fahd + 24);
// FADB so parsing reaches the paged check
let db = 0x100usize;
buf[db..db + 4].copy_from_slice(b"FADB");
@@ -486,6 +575,8 @@ mod tests {
buf[fahd + 7] = 10;
buf[fahd + 8..fahd + 16].copy_from_slice(&u64::MAX.to_le_bytes()); // absurd count
buf[fahd + 16..fahd + 24].copy_from_slice(&0x80u64.to_le_bytes());
// Valid checksum, so it is the element count that must be rejected.
stamp_checksum(&mut buf, fahd, fahd + 24);
buf[0x80..0x84].copy_from_slice(b"FADB");
let header = FixedArrayHeader::parse(&buf, fahd, 8, 8).unwrap();
let r = read_fixed_array_chunks(&buf, &header, &[100], &[20], 8, 8, 8);
@@ -545,6 +636,7 @@ mod tests {
file_data[fahd_offset + 8..fahd_offset + 16].copy_from_slice(&num_chunks.to_le_bytes());
file_data[fahd_offset + 16..fahd_offset + 24]
.copy_from_slice(&(db_offset as u64).to_le_bytes());
stamp_checksum(&mut file_data, fahd_offset, fahd_offset + 24);
// Build FADB at db_offset
file_data[db_offset..db_offset + 4].copy_from_slice(b"FADB");
@@ -562,6 +654,7 @@ mod tests {
let pos = elem_start + i * os;
file_data[pos..pos + os].copy_from_slice(&addr.to_le_bytes());
}
stamp_checksum(&mut file_data, db_offset, elem_start + 5 * os);
let header =
FixedArrayHeader::parse(&file_data, fahd_offset, offset_size, length_size).unwrap();
@@ -611,6 +704,7 @@ mod tests {
file_data[fahd_offset + 8..fahd_offset + 16].copy_from_slice(&num_chunks.to_le_bytes());
file_data[fahd_offset + 16..fahd_offset + 24]
.copy_from_slice(&(db_offset as u64).to_le_bytes());
stamp_checksum(&mut file_data, fahd_offset, fahd_offset + 24);
file_data[db_offset..db_offset + 4].copy_from_slice(b"FADB");
file_data[db_offset + 4] = 0;
@@ -632,6 +726,11 @@ mod tests {
file_data[pos + os..pos + os + 4].copy_from_slice(&csize.to_le_bytes());
file_data[pos + os + 4..pos + os + 8].copy_from_slice(&fmask.to_le_bytes());
}
stamp_checksum(
&mut file_data,
db_offset,
elem_start + test_chunks.len() * elem_size,
);
let header =
FixedArrayHeader::parse(&file_data, fahd_offset, offset_size, length_size).unwrap();
@@ -696,6 +795,7 @@ mod tests {
file_data[fahd_offset + 8..fahd_offset + 16].copy_from_slice(&num_elements.to_le_bytes());
file_data[fahd_offset + 16..fahd_offset + 24]
.copy_from_slice(&(db_offset as u64).to_le_bytes());
stamp_checksum(&mut file_data, fahd_offset, fahd_offset + 24);
// FADB prefix
file_data[db_offset..db_offset + 4].copy_from_slice(b"FADB");
@@ -715,6 +815,9 @@ mod tests {
let base_addr = 0x1000u64;
// Page 0 (elements 0..4) and page 2 (elements 8..11) carry addresses;
// page 1's slot is left zero-filled and must be skipped.
// The prefix and bitmap carry one checksum, each initialised page
// another — as a real file does.
stamp_checksum(&mut file_data, db_offset, bitmap_off + bitmap_size);
for &p in &[0usize, 2usize] {
let page_off = pages_start + p * page_total;
let count = core::cmp::min(page_nelmts, num_elements as usize - p * page_nelmts);
@@ -724,6 +827,7 @@ mod tests {
let pos = page_off + e * os;
file_data[pos..pos + os].copy_from_slice(&addr.to_le_bytes());
}
stamp_checksum(&mut file_data, page_off, page_off + count * os);
}
let header =