security: Tier 4a — bounds-check audit + new dataset-read fuzz target
CI / test (push) Failing after 4s

- Add ensure_len(data, offset, needed) helper to chunked_read.rs,
  data_read.rs, and local_heap.rs (matching the existing btree_v1.rs/
  object_header.rs convention) and use it at every plain-arithmetic
  offset+size bounds check found in these files, closing usize-overflow
  panics reachable from crafted near-usize::MAX offsets/addresses.
- collect_chunk_info: add a depth-limited internal wrapper
  (collect_chunk_info_inner, MAX_CHUNK_BTREE_DEPTH=64) to reject a
  crafted self-referencing/cyclic B-tree v1 chunk index instead of
  recursing unboundedly (stack-overflow DoS).
- read_compound_fields: validate byte_offset+field_size against the
  compound's declared element size before slicing, instead of an
  unguarded out-of-bounds panic on a crafted member offset.
- read_chunked_data/_cached/_sweep/_indexed: guard `ndims - 1` against
  underflow for a degenerate zero-dimension chunked layout.
- copy_chunk_to_output: rewrite all offset/stride arithmetic (both the
  1-D fast path and the general N-D path) to use checked_add/checked_mul,
  skipping an out-of-range row/chunk instead of panicking on overflow.

Add a new cargo-fuzz target, fuzz_dataset_read, that walks every dataset
in a parsed file via the clawhdf5 facade and exercises the contiguous/
chunked/compact raw-data read paths that the existing fuzz_full_file
target doesn't reach. Seeded with the chunked/VDS/compound-relevant test
fixtures plus two crash regressions found during this pass (the
copy_chunk_to_output overflow and the ndims-1 underflow, both fixed
above — this target found real bugs within the first couple of runs).
Not wired into CI (nightly-only, multi-minute runs); documented in
fuzz/README.md as a manual/scheduled check instead. Also fixed the
README's stale rustyhdf5-format naming while touching this file.

Added regression tests for every fix (near-usize::MAX offsets, the
self-referencing B-tree case, the compound byte_offset overrun, the
zero-dim layout, and both copy_chunk_to_output overflow paths) so these
are caught by `cargo test`, not just the fuzz corpus.
This commit is contained in:
Omar Sobh
2026-08-05 13:05:30 -07:00
parent a319405ffc
commit 297ee5ec17
21 changed files with 406 additions and 89 deletions
+250 -68
View File
@@ -61,12 +61,7 @@ fn decompress_all_chunks(
for chunk_info in chunks {
let c_addr = chunk_info.address as usize;
let size = chunk_info.chunk_size as usize;
if c_addr + size > file_data.len() {
return Err(FormatError::UnexpectedEof {
expected: c_addr + size,
available: file_data.len(),
});
}
ensure_len(file_data, c_addr, size)?;
let raw_chunk = &file_data[c_addr..c_addr + size];
let decompressed = if let Some(pl) = pipeline {
@@ -122,6 +117,21 @@ pub struct ChunkInfo {
pub address: u64,
}
/// Checks that `[offset, offset + needed)` fits within `data`, guarding the
/// addition against `usize` overflow from a crafted near-`usize::MAX` offset.
fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatError> {
if offset
.checked_add(needed)
.is_none_or(|end| end > data.len())
{
return Err(FormatError::UnexpectedEof {
expected: offset.saturating_add(needed),
available: data.len(),
});
}
Ok(())
}
fn read_offset(data: &[u8], pos: usize, size: u8) -> Result<u64, FormatError> {
let s = size as usize;
if pos.checked_add(s).is_none_or(|end| end > data.len()) {
@@ -150,19 +160,33 @@ pub fn collect_chunk_info(
btree_address: u64,
ndims: usize,
offset_size: u8,
_length_size: u8,
length_size: u8,
) -> Result<Vec<ChunkInfo>, FormatError> {
collect_chunk_info_inner(file_data, btree_address, ndims, offset_size, length_size, 0)
}
/// Maximum recursion depth for chunk B-tree traversal (malformed/cyclic data
/// protection), matching `btree_v1.rs`'s `MAX_BTREE_DEPTH`.
const MAX_CHUNK_BTREE_DEPTH: usize = 64;
fn collect_chunk_info_inner(
file_data: &[u8],
btree_address: u64,
ndims: usize,
offset_size: u8,
_length_size: u8,
depth: usize,
) -> Result<Vec<ChunkInfo>, FormatError> {
if depth > MAX_CHUNK_BTREE_DEPTH {
return Err(FormatError::NestingDepthExceeded);
}
let offset = btree_address as usize;
let os = offset_size as usize;
// Parse B-tree v1 header
let header_size = 8 + os * 2;
if offset + header_size > file_data.len() {
return Err(FormatError::UnexpectedEof {
expected: offset + header_size,
available: file_data.len(),
});
}
ensure_len(file_data, offset, header_size)?;
if &file_data[offset..offset + 4] != b"TREE" {
return Err(FormatError::InvalidBTreeSignature);
@@ -185,12 +209,7 @@ pub fn collect_chunk_info(
// Leaf node: keys and children interleaved
// key[0], child[0], key[1], child[1], ..., key[N-1], child[N-1], key[N]
let needed = entries_used * (key_size + os) + key_size;
if pos + needed > file_data.len() {
return Err(FormatError::UnexpectedEof {
expected: pos + needed,
available: file_data.len(),
});
}
ensure_len(file_data, pos, needed)?;
let mut chunks = Vec::with_capacity(entries_used);
for _ in 0..entries_used {
@@ -231,12 +250,7 @@ pub fn collect_chunk_info(
} else {
// Internal node: recurse into children
let needed = entries_used * (key_size + os) + key_size;
if pos + needed > file_data.len() {
return Err(FormatError::UnexpectedEof {
expected: pos + needed,
available: file_data.len(),
});
}
ensure_len(file_data, pos, needed)?;
let mut child_addrs = Vec::with_capacity(entries_used);
for _ in 0..entries_used {
@@ -248,8 +262,14 @@ pub fn collect_chunk_info(
let mut all_chunks = Vec::new();
for child_addr in child_addrs {
let child_chunks =
collect_chunk_info(file_data, child_addr, ndims, offset_size, _length_size)?;
let child_chunks = collect_chunk_info_inner(
file_data,
child_addr,
ndims,
offset_size,
_length_size,
depth + 1,
)?;
all_chunks.extend(child_chunks);
}
Ok(all_chunks)
@@ -347,7 +367,9 @@ pub fn read_chunked_data(
// Both v3 and v4 include element size as last dim (rank+1)
let ndims = chunk_dimensions.len();
let rank = ndims - 1;
let rank = ndims
.checked_sub(1)
.ok_or_else(|| FormatError::ChunkedReadError("chunked layout has no dimensions".into()))?;
let chunk_dims: Vec<usize> = chunk_dimensions[..rank]
.iter()
.map(|&d| d as usize)
@@ -461,12 +483,7 @@ pub fn read_chunked_data(
let c_addr = chunk_info.address as usize;
let size = chunk_info.chunk_size as usize;
if c_addr + size > file_data.len() {
return Err(FormatError::UnexpectedEof {
expected: c_addr + size,
available: file_data.len(),
});
}
ensure_len(file_data, c_addr, size)?;
let chunk_data = &file_data[c_addr..c_addr + size];
if rank == 0 {
@@ -579,7 +596,9 @@ pub fn read_chunked_data_cached(
let elem_size = datatype.type_size() as usize;
let ndims = chunk_dimensions.len();
let rank = ndims - 1;
let rank = ndims
.checked_sub(1)
.ok_or_else(|| FormatError::ChunkedReadError("chunked layout has no dimensions".into()))?;
let chunk_dims: Vec<usize> = chunk_dimensions[..rank]
.iter()
.map(|&d| d as usize)
@@ -697,12 +716,7 @@ pub fn read_chunked_data_cached(
// Decompress from file
let c_addr = chunk_info.address as usize;
let size = chunk_info.chunk_size as usize;
if c_addr + size > file_data.len() {
return Err(FormatError::UnexpectedEof {
expected: c_addr + size,
available: file_data.len(),
});
}
ensure_len(file_data, c_addr, size)?;
let raw_chunk = &file_data[c_addr..c_addr + size];
let dec = if let Some(pl) = pipeline {
if chunk_info.filter_mask == 0 {
@@ -935,7 +949,9 @@ pub fn read_chunked_data_sweep(
let elem_size = datatype.type_size() as usize;
let ndims = chunk_dimensions.len();
let rank = ndims - 1;
let rank = ndims
.checked_sub(1)
.ok_or_else(|| FormatError::ChunkedReadError("chunked layout has no dimensions".into()))?;
let chunk_dims: Vec<usize> = chunk_dimensions[..rank]
.iter()
.map(|&d| d as usize)
@@ -1062,12 +1078,7 @@ pub fn read_chunked_data_sweep(
// Decompress from file
let c_addr = chunk_info.address as usize;
let size = chunk_info.chunk_size as usize;
if c_addr + size > file_data.len() {
return Err(FormatError::UnexpectedEof {
expected: c_addr + size,
available: file_data.len(),
});
}
ensure_len(file_data, c_addr, size)?;
let raw_chunk = &file_data[c_addr..c_addr + size];
let dec = if let Some(pl) = pipeline {
if chunk_info.filter_mask == 0 {
@@ -1161,7 +1172,9 @@ pub fn read_chunked_data_indexed(
let elem_size = datatype.type_size() as usize;
let ndims = chunk_dimensions.len();
let rank = ndims - 1;
let rank = ndims
.checked_sub(1)
.ok_or_else(|| FormatError::ChunkedReadError("chunked layout has no dimensions".into()))?;
let chunk_dims: Vec<usize> = chunk_dimensions[..rank]
.iter()
.map(|&d| d as usize)
@@ -1278,12 +1291,7 @@ pub fn read_chunked_data_indexed(
} else {
let c_addr = *file_offset as usize;
let size = *file_size as usize;
if c_addr + size > file_data.len() {
return Err(FormatError::UnexpectedEof {
expected: c_addr + size,
available: file_data.len(),
});
}
ensure_len(file_data, c_addr, size)?;
let raw_chunk = &file_data[c_addr..c_addr + size];
let decompressed = if let Some(pl) = pipeline {
if *filter_mask == 0 {
@@ -1331,9 +1339,18 @@ fn copy_chunk_to_output(
// Fast path for 1-D: single contiguous copy per chunk
let global_start = chunk_offsets[0];
let copy_len = chunk_dims[0].min(ds_dims[0].saturating_sub(global_start));
let src_bytes = copy_len * elem_size;
let dst_start = global_start * elem_size;
if src_bytes > 0 && dst_start + src_bytes <= output.len() && src_bytes <= chunk_data.len() {
let (Some(src_bytes), Some(dst_start)) = (
copy_len.checked_mul(elem_size),
global_start.checked_mul(elem_size),
) else {
return;
};
if src_bytes > 0
&& dst_start
.checked_add(src_bytes)
.is_some_and(|end| end <= output.len())
&& src_bytes <= chunk_data.len()
{
output[dst_start..dst_start + src_bytes].copy_from_slice(&chunk_data[..src_bytes]);
}
return;
@@ -1343,19 +1360,29 @@ fn copy_chunk_to_output(
let inner_dim = rank - 1;
let inner_chunk_len =
chunk_dims[inner_dim].min(ds_dims[inner_dim].saturating_sub(chunk_offsets[inner_dim]));
let row_bytes = inner_chunk_len * elem_size;
let Some(row_bytes) = inner_chunk_len.checked_mul(elem_size) else {
return;
};
if row_bytes == 0 {
return;
}
// Number of rows = product of all outer chunk dimensions
let outer_count: usize = chunk_dims[..inner_dim].iter().product();
let Some(outer_count) = chunk_dims[..inner_dim]
.iter()
.try_fold(1usize, |acc, &d| acc.checked_mul(d))
else {
return;
};
// Outer strides for iterating chunk-local coordinates
let mut outer_strides = vec![1usize; inner_dim];
for i in (0..inner_dim.saturating_sub(1)).rev() {
outer_strides[i] = outer_strides[i + 1] * chunk_dims[i + 1];
let Some(stride) = outer_strides[i + 1].checked_mul(chunk_dims[i + 1]) else {
return;
};
outer_strides[i] = stride;
}
for outer_idx in 0..outer_count {
@@ -1375,13 +1402,29 @@ fn copy_chunk_to_output(
remaining %= outer_strides[d];
}
let global_coord = chunk_offsets[d] + coord_in_chunk;
let Some(global_coord) = chunk_offsets[d].checked_add(coord_in_chunk) else {
out_of_bounds = true;
break;
};
if global_coord >= ds_dims[d] {
out_of_bounds = true;
break;
}
ds_flat += global_coord * ds_strides[d];
src_flat += coord_in_chunk * chunk_strides[d];
let (Some(ds_term), Some(src_term)) = (
global_coord.checked_mul(ds_strides[d]),
coord_in_chunk.checked_mul(chunk_strides[d]),
) else {
out_of_bounds = true;
break;
};
let (Some(new_ds_flat), Some(new_src_flat)) =
(ds_flat.checked_add(ds_term), src_flat.checked_add(src_term))
else {
out_of_bounds = true;
break;
};
ds_flat = new_ds_flat;
src_flat = new_src_flat;
}
if out_of_bounds {
@@ -1389,12 +1432,27 @@ fn copy_chunk_to_output(
}
// Add innermost dimension offset
ds_flat += chunk_offsets[inner_dim] * ds_strides[inner_dim];
let Some(inner_term) = chunk_offsets[inner_dim].checked_mul(ds_strides[inner_dim]) else {
continue;
};
let Some(ds_flat) = ds_flat.checked_add(inner_term) else {
continue;
};
let src_start = src_flat * elem_size;
let dst_start = ds_flat * elem_size;
let (Some(src_start), Some(dst_start)) = (
src_flat.checked_mul(elem_size),
ds_flat.checked_mul(elem_size),
) else {
continue;
};
if src_start + row_bytes <= chunk_data.len() && dst_start + row_bytes <= output.len() {
let fits = src_start
.checked_add(row_bytes)
.is_some_and(|end| end <= chunk_data.len())
&& dst_start
.checked_add(row_bytes)
.is_some_and(|end| end <= output.len());
if fits {
output[dst_start..dst_start + row_bytes]
.copy_from_slice(&chunk_data[src_start..src_start + row_bytes]);
}
@@ -1639,6 +1697,82 @@ mod tests {
(file_data, layout, dataspace)
}
#[test]
fn read_chunked_data_rejects_zero_dim_chunk_layout() {
// Found by fuzzing: chunk_dimensions.len() == 0 caused `ndims - 1` to
// underflow. A malformed/degenerate chunked layout must error cleanly.
let layout = DataLayout::Chunked {
chunk_dimensions: vec![],
btree_address: Some(0),
version: 3,
chunk_index_type: None,
single_chunk_filtered_size: None,
single_chunk_filter_mask: None,
};
let dataspace = Dataspace {
space_type: DataspaceType::Simple,
rank: 1,
dimensions: vec![10],
max_dimensions: None,
};
let datatype = make_f64_type();
let file_data = vec![0u8; 64];
let result = read_chunked_data(&file_data, &layout, &dataspace, &datatype, None, 8, 8);
assert!(
matches!(result, Err(FormatError::ChunkedReadError(_))),
"expected a clean ChunkedReadError, got {result:?}"
);
}
#[test]
fn copy_chunk_to_output_1d_rejects_overflowing_offset_without_panicking() {
// Found by fuzzing: `global_start * elem_size` overflowed for a
// crafted large chunk offset.
let chunk_data = vec![1u8; 16];
let mut output = vec![0u8; 16];
let chunk_offsets = [usize::MAX - 1];
let chunk_dims = [1usize];
let ds_dims = [usize::MAX];
let ds_strides = [1usize];
let chunk_strides = [1usize];
copy_chunk_to_output(
&chunk_data,
&mut output,
&chunk_offsets,
&chunk_dims,
&ds_dims,
&ds_strides,
&chunk_strides,
8,
1,
);
// No panic; the out-of-range write was skipped, output left untouched.
assert_eq!(output, vec![0u8; 16]);
}
#[test]
fn copy_chunk_to_output_nd_rejects_overflowing_offset_without_panicking() {
let chunk_data = vec![1u8; 16];
let mut output = vec![0u8; 16];
let chunk_offsets = [usize::MAX - 1, 0];
let chunk_dims = [1usize, 1usize];
let ds_dims = [usize::MAX, usize::MAX];
let ds_strides = [1usize, 1usize];
let chunk_strides = [1usize, 1usize];
copy_chunk_to_output(
&chunk_data,
&mut output,
&chunk_offsets,
&chunk_dims,
&ds_dims,
&ds_strides,
&chunk_strides,
8,
2,
);
assert_eq!(output, vec![0u8; 16]);
}
#[test]
fn read_1d_two_chunks_no_compression() {
let values: Vec<f64> = (0..20).map(|i| i as f64).collect();
@@ -1851,6 +1985,54 @@ mod tests {
assert_eq!(err, FormatError::InvalidBTreeNodeType(0));
}
#[test]
fn collect_chunk_info_rejects_near_usize_max_offset() {
let file_data = vec![0u8; 64];
let result = collect_chunk_info(&file_data, u64::MAX - 4, 2, 8, 8);
assert!(
matches!(result, Err(FormatError::UnexpectedEof { .. })),
"expected a clean UnexpectedEof, got {result:?}"
);
}
#[test]
fn collect_chunk_info_rejects_self_referencing_internal_node() {
// A type-1 internal node (level 1) whose single child address points
// back to itself: an infinite-recursion / cyclic B-tree attack.
let ndims = 2;
let os: u8 = 8;
let mut buf = Vec::new();
buf.extend_from_slice(b"TREE");
buf.push(1); // node_type = 1 (raw data chunks)
buf.push(1); // node_level = 1 (internal)
buf.extend_from_slice(&1u16.to_le_bytes()); // entries_used = 1
write_offset(&mut buf, u64::MAX, os); // left sibling undefined
write_offset(&mut buf, u64::MAX, os); // right sibling undefined
// key[0]: chunk_size(4) + filter_mask(4) + ndims offsets
buf.extend_from_slice(&0u32.to_le_bytes());
buf.extend_from_slice(&0u32.to_le_bytes());
for _ in 0..ndims {
write_offset(&mut buf, 0, os);
}
// child[0]: points back to offset 0 (this same node) — cyclic.
write_offset(&mut buf, 0, os);
// final key
buf.extend_from_slice(&0u32.to_le_bytes());
buf.extend_from_slice(&0u32.to_le_bytes());
for _ in 0..ndims {
write_offset(&mut buf, u64::MAX, os);
}
let mut file_data = vec![0u8; 256];
file_data[..buf.len()].copy_from_slice(&buf);
let result = collect_chunk_info(&file_data, 0, ndims, os, os);
assert!(
matches!(result, Err(FormatError::NestingDepthExceeded)),
"expected a clean NestingDepthExceeded, got {result:?}"
);
}
// --- Implicit chunk generation tests ---
#[test]