diff --git a/CHANGELOG.md b/CHANGELOG.md index da4972d..e5bd339 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ ## Unreleased +### Integrity +- `clawhdf5-format`: **Fixed and Extensible Array chunk indexes now verify + their checksums** (the `checksum` feature, on by default). Every structure + in both — header, index block, super block, data block and each data block + page — carries a Jenkins lookup3 checksum that was parsed past and ignored. + The consequence of skipping it is not a missing warning but wrong data: a + single flipped bit in a chunk address still parses, still points inside the + file, and the reader hands back whatever bytes now sit there as the chunk's + contents. Verified in both directions — the checksums accept files written + by HDF5 2.0 at 100 to 200 000 chunks, dense, sparse, filtered and paged, + and an interop test corrupts an address to confirm the read now fails + instead of returning data (it does return data when the check is removed). + ### Performance - `clawhdf5-accel`: **`dot_i8`, a runtime-dispatched int8 dot product** (AVX2: sign-extend each half to `i16`, then `madd_epi16`; scalar fallback diff --git a/crates/clawhdf5-format/src/extensible_array.rs b/crates/clawhdf5-format/src/extensible_array.rs index c3665d2..4416a76 100644 --- a/crates/clawhdf5-format/src/extensible_array.rs +++ b/crates/clawhdf5-format/src/extensible_array.rs @@ -12,6 +12,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 Extensible Array structure carries one. +/// +/// A corrupt chunk index yields addresses pointing at the wrong bytes, so a +/// mismatch is an error: otherwise the damage surfaces as plausible data read +/// 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 Extensible Array header (AEHD). #[derive(Debug, Clone)] pub struct ExtensibleArrayHeader { @@ -145,6 +170,8 @@ impl ExtensibleArrayHeader { pos += ls; // skip nelmts pos += ls; // skip max_idx_set (6th stats field) let index_block_address = read_offset(d, pos, offset_size)?; + pos += offset_size as usize; + verify_checksum(file_data, offset, offset + pos)?; Ok(ExtensibleArrayHeader { client_id, @@ -361,6 +388,17 @@ fn read_data_block_elements( }; if nelmts <= page { + // Prefix and elements are covered by one checksum. + let elem_bytes = if header.client_id == 0 { + offset_size as usize + } else { + header.element_size as usize + }; + let end = nelmts + .checked_mul(elem_bytes) + .and_then(|b| pos.checked_add(b)) + .ok_or_else(|| FormatError::Overflow("Extensible Array data block span".into()))?; + verify_checksum(file_data, db_offset, end)?; read_run(pos, nelmts, start_index, &mut chunks)?; return Ok(chunks); } @@ -369,6 +407,7 @@ fn read_data_block_elements( // each holding `page` elements followed by a checksum. Pages whose bit is // clear were never written; their slot still occupies the file, so stride // over it rather than reading zeros as addresses. + verify_checksum(file_data, db_offset, pos)?; pos += 4; let elem_bytes = if header.client_id == 0 { offset_size as usize @@ -389,6 +428,9 @@ fn read_data_block_elements( .is_some_and(|byte| byte & (0x80 >> (bit % 8)) != 0); if initialised { let count = core::cmp::min(page, nelmts - p * page); + // Each page carries its own checksum, over a full page's worth of + // slots even when the last one holds fewer live elements. + verify_checksum(file_data, pos, pos + page * elem_bytes)?; read_run(pos, count, start_index + p * page, &mut chunks)?; } pos = pos @@ -447,6 +489,63 @@ pub fn read_extensible_array_chunks( let mut chunks = Vec::new(); let total_elements = header.num_elements as usize; + let dmin = header.min_dblk_nelmts as usize; + if dmin == 0 || !dmin.is_power_of_two() { + return Err(FormatError::ChunkedReadError( + "Extensible Array data block minimum is not a power of two".into(), + )); + } + // nsblks = 1 + (max_nelmts_bits - log2(data_blk_min_elmts)), and the index + // block holds 2 * (sup_blk_min_data_ptrs - 1) data block addresses. + let log2_dmin = dmin.trailing_zeros() as usize; + let nsblks = 1 + (header.max_nelmts_bits as usize).saturating_sub(log2_dmin); + let ndblk_addrs = 2 * (header.super_blk_min_nelmts as usize).saturating_sub(1); + + // The data blocks listed directly in the index block are the first + // `ndblk_addrs` in super-block order, each sized by the level it belongs + // to; the super block addresses that follow resume at the next level. + let mut direct: Vec = Vec::with_capacity(ndblk_addrs); + let mut level = 0usize; + while direct.len() < ndblk_addrs { + if level >= nsblks { + return Err(FormatError::ChunkedReadError( + "Extensible Array index block claims more data blocks than the array has".into(), + )); + } + let (ndblks, dblk_nelmts) = sblk_info(level, dmin).ok_or_else(|| { + FormatError::Overflow("Extensible Array super block layout overflows usize".into()) + })?; + for _ in 0..ndblks { + direct.push(dblk_nelmts); + } + level += 1; + } + if direct.len() != ndblk_addrs { + // A partial level in the index block is not a layout HDF5 produces, + // and guessing where the super blocks resume would misplace elements. + return Err(FormatError::ChunkedReadError( + "Extensible Array index block ends mid super block".into(), + )); + } + + // One checksum covers the prefix, every inline element slot, and every + // data block and super block address. + let elem_bytes = if header.client_id == 0 { + os + } else { + header.element_size as usize + }; + let ib_end = (header.idx_blk_elmts as usize) + .checked_mul(elem_bytes) + .and_then(|b| pos.checked_add(b)) + .and_then(|p| { + ndblk_addrs + .checked_add(nsblks - level) + .and_then(|n| n.checked_mul(os).and_then(|b| p.checked_add(b))) + }) + .ok_or_else(|| FormatError::Overflow("Extensible Array index block span".into()))?; + verify_checksum(file_data, ib_offset, ib_end)?; + // 1. Elements stored inline in the index block. let n_inline = (header.idx_blk_elmts as usize).min(total_elements); for i in 0..n_inline { @@ -471,45 +570,7 @@ pub fn read_extensible_array_chunks( return Ok(chunks); } - let dmin = header.min_dblk_nelmts as usize; - if dmin == 0 || !dmin.is_power_of_two() { - return Err(FormatError::ChunkedReadError( - "Extensible Array data block minimum is not a power of two".into(), - )); - } - // nsblks = 1 + (max_nelmts_bits - log2(data_blk_min_elmts)), and the index - // block holds 2 * (sup_blk_min_data_ptrs - 1) data block addresses. - let log2_dmin = dmin.trailing_zeros() as usize; - let nsblks = 1 + (header.max_nelmts_bits as usize).saturating_sub(log2_dmin); - let ndblk_addrs = 2 * (header.super_blk_min_nelmts as usize).saturating_sub(1); - - // 2. Data blocks listed directly in the index block: the first - // `ndblk_addrs` data blocks in super-block order, each sized by the - // level it belongs to. - let mut direct: Vec = Vec::with_capacity(ndblk_addrs); - let mut level = 0usize; - while direct.len() < ndblk_addrs { - let (ndblks, dblk_nelmts) = sblk_info(level, dmin).ok_or_else(|| { - FormatError::Overflow("Extensible Array super block layout overflows usize".into()) - })?; - if level >= nsblks { - return Err(FormatError::ChunkedReadError( - "Extensible Array index block claims more data blocks than the array has".into(), - )); - } - for _ in 0..ndblks { - direct.push(dblk_nelmts); - } - level += 1; - } - if direct.len() != ndblk_addrs { - // A partial level in the index block is not a layout HDF5 produces, - // and guessing where the super blocks resume would misplace elements. - return Err(FormatError::ChunkedReadError( - "Extensible Array index block ends mid super block".into(), - )); - } - + // 2. Data blocks listed directly in the index block. for &dblk_nelmts in &direct { if global_index >= total_elements { return Ok(chunks); @@ -629,6 +690,13 @@ fn read_super_block( let mut chunks = Vec::new(); let mut global_idx = start_index; + // One checksum covers the prefix, the bitmap and every data block address. + let sb_end = ndblks + .checked_mul(os) + .and_then(|b| pos.checked_add(b)) + .ok_or_else(|| FormatError::Overflow("Extensible Array super block span".into()))?; + verify_checksum(file_data, sb_offset, sb_end)?; + for i in 0..ndblks { ensure_len(file_data, pos, os)?; let addr = read_offset(file_data, pos, offset_size)?; @@ -657,6 +725,14 @@ fn read_super_block( #[cfg(test)] mod tests { use super::*; + + /// Stamp the Jenkins checksum a real file would carry over + /// `data[start..end]`, writing it at `end`. Hand-built fixtures need this + /// now that the reader validates it, exactly as HDF5 writes it. + 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]; @@ -712,6 +788,7 @@ mod tests { buf[44..52].copy_from_slice(&5u64.to_le_bytes()); // stat[4] = num_elements buf[52..60].copy_from_slice(&0u64.to_le_bytes()); // stat[5] buf[60..68].copy_from_slice(&0x1000u64.to_le_bytes()); // index_block_address + stamp_checksum(&mut buf, 0, 68); let hdr = ExtensibleArrayHeader::parse(&buf, 0, os, ls).unwrap(); assert_eq!(hdr.client_id, 0); @@ -797,6 +874,7 @@ mod tests { .copy_from_slice(&(num_chunks as u64).to_le_bytes()); file_data[aehd_offset + 60..aehd_offset + 68] .copy_from_slice(&(aeib_offset as u64).to_le_bytes()); + stamp_checksum(&mut file_data, aehd_offset, aehd_offset + 68); // checksum (4 bytes at +68) — not validated // Build AEIB at aeib_offset @@ -814,6 +892,23 @@ mod tests { let p = elem_start + i * osv; file_data[p..p + osv].copy_from_slice(&addr.to_le_bytes()); } + // The index block's checksum covers its prefix, every inline element + // slot, and every data block and super block address slot: + // ndblk_addrs = 2 * (sup_blk_min_data_ptrs - 1), and the super block + // pointers make up the rest of nsblks levels. + let sup_ptrs = file_data[aehd_offset + 10] as usize; + let dmin = file_data[aehd_offset + 9] as usize; + let nsblks = 1 + 10 - dmin.trailing_zeros() as usize; + let ndblk_addrs = 2 * (sup_ptrs - 1); + // Levels consumed by those direct data blocks (1, 1, 2, 2, ... per level). + let mut consumed = 0usize; + let mut levels = 0usize; + while consumed < ndblk_addrs { + consumed += 1 << (levels / 2); + levels += 1; + } + let ib_end = elem_start + num_chunks * osv + (ndblk_addrs + nsblks - levels) * osv; + stamp_checksum(&mut file_data, aeib_offset, ib_end); let header = ExtensibleArrayHeader::parse(&file_data, aehd_offset, os, ls).unwrap(); let ds_dims = vec![40u64]; // 2 chunks × 20 elements @@ -863,6 +958,7 @@ mod tests { // idx_blk_addr at offset 12 + 6*8 = 60 file_data[aehd_offset + 60..aehd_offset + 68] .copy_from_slice(&(aeib_offset as u64).to_le_bytes()); + stamp_checksum(&mut file_data, aehd_offset, aehd_offset + 68); // AEIB file_data[aeib_offset..aeib_offset + 4].copy_from_slice(b"EAIB"); @@ -881,42 +977,48 @@ mod tests { pos += osv; } - // Direct data block addresses: first sb_level=0 has 1 dblk, sb_level=1 has 1 dblk - // Total direct dblks for sblk_min=2: 2^0 + 2^1 = 1 + 2 = 3 (oops) - // Actually: sblk_min levels. level 0: 2^0=1 dblk, level 1: 2^1=2 dblks => 3 dblks - // But we only have 2 remaining elements. - // dblk sizes: level 0: 1 dblk of min_dblk=2; level 1: 2 dblks of 2 each (nelmts doubles at level > 0) - // Wait, re-reading the code: at level 0, nelmts=min_dblk=2, 1 dblk. - // At level 1, 1 dblk, nelmts still 2 (doubles only at level > 0... but the code says - // `if sb_level > 0 { nelmts *= 2 }` after pushing). Let me re-check. - // After push at level 0: nelmts=2. Then if 0>0 false, no double. Push 1 dblk of 2. - // Level 1: ndblks=2. Push 2 dblks of 2. Then 1>0 true, nelmts=4. - // Total: 3 dblks with sizes [2, 2, 2]. Total = 6. - // We only need 2 more elements. So only the first dblk has data. - let n_direct_dblks = 3; + // Direct data block addresses. With sup_blk_min_data_ptrs = 2 the index + // block holds 2 * (2 - 1) = 2 of them, which are the data blocks of + // super block levels 0 and 1: one of `min_dblk_nelmts` elements, then + // one of twice that (ndblks = 2^(u/2), dblk_nelmts = 2^((u+1)/2) * min). + // Only the first is allocated here; the rest of the array is empty. + let ndblk_addrs = 2 * (sblk_min as usize - 1); file_data[pos..pos + osv].copy_from_slice(&(aedb_offset as u64).to_le_bytes()); pos += osv; - // 2 more dblk addresses - undefined - for _ in 1..n_direct_dblks { + for _ in 1..ndblk_addrs { file_data[pos..pos + osv].copy_from_slice(&u64::MAX.to_le_bytes()); pos += osv; } + // Super block addresses fill the remaining levels; all unallocated. + let nsblks = 1 + 10 - (min_dblk_nelmts as usize).trailing_zeros() as usize; + let mut consumed = 0usize; + let mut levels = 0usize; + while consumed < ndblk_addrs { + consumed += 1 << (levels / 2); + levels += 1; + } + for _ in 0..(nsblks - levels) { + file_data[pos..pos + osv].copy_from_slice(&u64::MAX.to_le_bytes()); + pos += osv; + } + stamp_checksum(&mut file_data, aeib_offset, pos); - // EADB at aedb_offset (min_dblk_nelmts elements) + // EADB holding the first data block's `min_dblk_nelmts` elements. file_data[aedb_offset..aedb_offset + 4].copy_from_slice(b"EADB"); file_data[aedb_offset + 4] = 0; file_data[aedb_offset + 5] = 0; file_data[aedb_offset + 6..aedb_offset + 14] .copy_from_slice(&(aehd_offset as u64).to_le_bytes()); - // block_offset: ceil(max_nelmts_bits/8) = ceil(10/8) = 2 bytes - // block_offset = 0 for first data block - let blk_off_size = (10usize).div_ceil(8); // max_nelmts_bits=10 - let mut dbpos = aedb_offset + 6 + osv + blk_off_size; + // Block offset field: ceil(max_nelmts_bits / 8) bytes, zero here. + let blk_off_size = (10usize).div_ceil(8); + let db_elems = aedb_offset + 6 + osv + blk_off_size; + let mut dbpos = db_elems; for i in 0..min_dblk_nelmts as usize { let addr = base_addr + (idx_blk_elmts as u64 + i as u64) * chunk_byte_size; file_data[dbpos..dbpos + osv].copy_from_slice(&addr.to_le_bytes()); dbpos += osv; } + stamp_checksum(&mut file_data, aedb_offset, dbpos); let header = ExtensibleArrayHeader::parse(&file_data, aehd_offset, os, ls).unwrap(); let ds_dims = vec![40u64]; diff --git a/crates/clawhdf5-format/src/fixed_array.rs b/crates/clawhdf5-format/src/fixed_array.rs index 4b6543f..4c79f03 100644 --- a/crates/clawhdf5-format/src/fixed_array.rs +++ b/crates/clawhdf5-format/src/fixed_array.rs @@ -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 { 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, 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, 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 = diff --git a/crates/clawhdf5/tests/h5py_interop_tests.rs b/crates/clawhdf5/tests/h5py_interop_tests.rs index 7082a06..370b64e 100644 --- a/crates/clawhdf5/tests/h5py_interop_tests.rs +++ b/crates/clawhdf5/tests/h5py_interop_tests.rs @@ -1318,3 +1318,72 @@ with h5py.File("{sparse_str}", "w", libver="latest") as f: .count(); assert_eq!(wrong, 0, "sparse: {wrong} of {n} elements read back wrong"); } + +#[test] +fn corrupting_a_chunk_index_is_an_error_not_wrong_data() { + // Every Fixed/Extensible Array structure carries a Jenkins checksum, and + // the reader now verifies it. The point is not the checksum itself but + // what it prevents: a damaged index otherwise yields addresses pointing + // at the wrong bytes, and the caller receives another chunk's data as if + // it were the one asked for. + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + + for (name, maxshape) in [("fixed", "None"), ("extensible", "(None,)")] { + let path = dir.path().join(format!("{name}.h5")); + let path_str = path.display().to_string(); + let shape_arg = if maxshape == "None" { + String::new() + } else { + format!(", maxshape={maxshape}") + }; + run_python(&format!( + r#" +import h5py, numpy as np +with h5py.File("{path_str}", "w", libver="latest") as f: + d = f.create_dataset("x", shape=(400,), chunks=(1,), dtype="i4"{shape_arg}) + d[...] = np.arange(400, dtype="i4") +"# + )); + + let clean = std::fs::read(&path).unwrap(); + assert_eq!( + File::open(&path) + .unwrap() + .dataset("x") + .unwrap() + .read_i32() + .unwrap() + .len(), + 400, + "{name}: the intact file must read" + ); + + // Flip a low bit of a chunk address inside a data block. Structurally + // everything still parses — the index still has the right shape and + // the address still lands inside the file — so nothing but the + // checksum can notice. Without it the read succeeds and hands back + // whatever bytes now sit at that address. + let sig: &[u8] = if name == "fixed" { b"FADB" } else { b"EADB" }; + let block = clean + .windows(4) + .position(|w| w == sig) + .unwrap_or_else(|| panic!("{name}: no data block in the fixture")); + // Past the prefix (signature, version, client id, header address, and + // for the Extensible Array a block offset), into the first address. + let at = block + 4 + 1 + 1 + 8 + if name == "fixed" { 0 } else { 4 } + 1; + let mut damaged = clean.clone(); + damaged[at] ^= 0x10; + let damaged_path = dir.path().join(format!("{name}_damaged.h5")); + std::fs::write(&damaged_path, &damaged).unwrap(); + + let result = File::open(&damaged_path) + .unwrap() + .dataset("x") + .and_then(|d| d.read_i32()); + assert!( + result.is_err(), + "{name}: corruption produced data instead of an error" + ); + } +}