fix(format): bound B-tree v2 traversal against crafted files

Traversal recursed one frame per level with the depth taken from the
file (a u16), and followed child addresses without asking whether they
were shared. Two crafted inputs, both reproduced before fixing:

- A node listing itself as its own child, under a header claiming 65 535
  levels, overflowed the stack and aborted the process — SIGABRT, not an
  error a caller can handle — from under 100 bytes.
- Levels whose children all point at one shared node below reached it
  fan-out^depth times: 29.5 million records in 8 s from ~5 KB, and one
  more level would exhaust memory.

Depth is now capped at 64, as the fractal heap already was; no real tree
approaches it, since even at the minimum fan-out of two that is over
2^64 records. And traversal stops once it has produced more records
than the file has bytes to hold them — a valid tree stores each record
once in its own bytes, so this bounds shared subtrees without trusting
the header's own `total_records`. Both inputs now fail in under a
millisecond.

Every B-tree v2 user goes through this collector: dense attributes, v2
groups, shared messages and chunk indexes. To show the budget never
refuses a real file, a new interop test has HDF5 2.0 write a depth-2
chunk index with 40 000 records and reads back all 160 000 values; it
fails when the budget is deliberately made too tight.

Also corrects `BM25Index::search`, which claimed to use Block-Max WAND.
It scores exhaustively, and pruning would not help the store:
`hybrid_search` needs every score because fusion normalises over them.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-20 16:56:05 -07:00
co-authored by Claude Opus 5
parent 5889b378e9
commit e9aeb110b7
5 changed files with 251 additions and 4 deletions
+24
View File
@@ -1,5 +1,29 @@
# Changelog # Changelog
## Unreleased
### Security
- `clawhdf5-format`: **a crafted file could crash any reader through B-tree v2
traversal.** Recursion was bounded only by the depth the file claimed (a
`u16`), and child addresses were never checked for sharing. A node listing
itself as its own child under a header claiming 65 535 levels — under 100
bytes — overflowed the stack and **aborted the process** (SIGABRT, not a
catchable error). Levels whose children all point at one shared node below
reached it fan-out^depth times: 29.5 million records from ~5 KB, and one
more level would exhaust memory. Both are now errors, returned in under a
millisecond: depth is capped at 64 (as the fractal heap already was), and
traversal stops once it has produced more records than the file has bytes
to hold. Every B-tree v2 user goes through this path — dense attributes,
v2 groups, shared messages and chunk indexes. Valid files are unaffected,
including a depth-2 HDF5 2.0 chunk index with 40 000 records, now covered by
an interop test.
### Documentation
- `clawhdf5-agent`: `BM25Index::search` claimed to use Block-Max WAND for early
termination. It never did; it scores every match exhaustively. It now says
so, and why no pruning would help the store: `hybrid_search` uses `scores()`,
since fusion normalises over every match.
## v2.6.0 (2026-09-20) ## v2.6.0 (2026-09-20)
### Upgrade Notes ### Upgrade Notes
+8 -4
View File
@@ -88,8 +88,11 @@ impl BM25Index {
/// Search the index for a query, returning the top `k` results /// Search the index for a query, returning the top `k` results
/// as `(doc_id, score)` pairs sorted by score descending. /// as `(doc_id, score)` pairs sorted by score descending.
/// ///
/// Uses Block-Max WAND for early termination when remaining documents /// Scores every matching document exhaustively, then keeps the top `k`.
/// cannot beat the current top-k threshold. /// There is no early termination (WAND, MaxScore): the store's hot path
/// is [`scores`](Self::scores), because score fusion normalises over the
/// whole matching set and so needs every score, which no pruning scheme
/// can skip. This method is for BM25-only callers.
pub fn search(&self, query: &str, k: usize) -> Vec<(usize, f32)> { pub fn search(&self, query: &str, k: usize) -> Vec<(usize, f32)> {
if k == 0 { if k == 0 {
return Vec::new(); return Vec::new();
@@ -561,8 +564,9 @@ mod tests {
} }
#[test] #[test]
fn wand_returns_same_results_as_exhaustive() { fn top_k_search_matches_ranking_every_score() {
// WAND-style search should produce same scores as exhaustive // `search` must agree with ranking the full `scores` set — the
// bounded heap is an optimisation over sorting, not an approximation.
let docs: Vec<String> = (0..100) let docs: Vec<String> = (0..100)
.map(|i| { .map(|i| {
if i % 3 == 0 { if i % 3 == 0 {
+151
View File
@@ -172,6 +172,17 @@ fn max_records_leaf(node_size: u32, record_size: u16) -> u64 {
((node_size - overhead) / record_size as u32) as u64 ((node_size - overhead) / record_size as u32) as u64
} }
/// Deepest B-tree v2 accepted. See [`collect_btree_v2_records`].
const MAX_DEPTH: u16 = 64;
/// Take `n` records from the traversal's budget, or refuse the tree.
fn spend(budget: &mut usize, n: usize) -> Result<(), FormatError> {
*budget = budget
.checked_sub(n)
.ok_or(FormatError::NestingDepthExceeded)?;
Ok(())
}
/// Collect all records from a B-tree v2 by traversing from the root. /// Collect all records from a B-tree v2 by traversing from the root.
pub fn collect_btree_v2_records( pub fn collect_btree_v2_records(
file_data: &[u8], file_data: &[u8],
@@ -182,6 +193,22 @@ pub fn collect_btree_v2_records(
if header.total_records == 0 || header.num_records_in_root == 0 { if header.total_records == 0 || header.num_records_in_root == 0 {
return Ok(Vec::new()); return Ok(Vec::new());
} }
// Recursion is one frame per level, and the depth is read from the file:
// a crafted header claiming 65 535 levels over a node that is its own
// child overflowed the stack. 64 matches the fractal heap's guard, and no
// real tree comes close — even at the minimum fan-out of two it would
// hold more than 2^64 records.
if header.depth > MAX_DEPTH {
return Err(FormatError::NestingDepthExceeded);
}
// A valid tree stores each record once, in its own bytes, so it cannot
// hold more records than the file has room for. Children are addresses,
// though, and nothing makes them distinct: levels whose children all
// point at one shared node below reach it fan-out^depth times, which is
// millions of records from a few kilobytes. Counting against what the
// file could physically contain bounds that without trusting the
// header's own `total_records`.
let mut budget = file_data.len() / usize::from(header.record_size.max(1));
let max_leaf_nrec = max_records_leaf(header.node_size, header.record_size); let max_leaf_nrec = max_records_leaf(header.node_size, header.record_size);
@@ -206,6 +233,7 @@ pub fn collect_btree_v2_records(
offset_size, offset_size,
length_size, length_size,
max_leaf_nrec, max_leaf_nrec,
&mut budget,
&mut records, &mut records,
)?; )?;
Ok(records) Ok(records)
@@ -273,6 +301,7 @@ fn collect_internal_records(
offset_size: u8, offset_size: u8,
length_size: u8, length_size: u8,
max_leaf_nrec: u64, max_leaf_nrec: u64,
budget: &mut usize,
out: &mut Vec<BTreeV2Record>, out: &mut Vec<BTreeV2Record>,
) -> Result<(), FormatError> { ) -> Result<(), FormatError> {
// signature(4) + version(1) + type(1) = 6 // signature(4) + version(1) + type(1) = 6
@@ -350,6 +379,8 @@ fn collect_internal_records(
// We collect child[0] records, then record[0], then child[1], etc. // We collect child[0] records, then record[0], then child[1], etc.
for (i, &(child_addr, child_nrec)) in children.iter().enumerate() { for (i, &(child_addr, child_nrec)) in children.iter().enumerate() {
if child_depth == 0 { if child_depth == 0 {
// Before parsing, so a refused tree is not also a large allocation.
spend(budget, usize::from(child_nrec))?;
let leaf_recs = let leaf_recs =
parse_leaf_records(file_data, child_addr as usize, child_nrec, record_size)?; parse_leaf_records(file_data, child_addr as usize, child_nrec, record_size)?;
out.extend(leaf_recs); out.extend(leaf_recs);
@@ -364,6 +395,7 @@ fn collect_internal_records(
offset_size, offset_size,
length_size, length_size,
max_leaf_nrec, max_leaf_nrec,
budget,
out, out,
)?; )?;
} }
@@ -393,6 +425,7 @@ fn collect_internal_records(
available: file_data.len(), available: file_data.len(),
}); });
} }
spend(budget, 1)?;
out.push(BTreeV2Record { out.push(BTreeV2Record {
data: file_data[rec_start..rec_end].to_vec(), data: file_data[rec_start..rec_end].to_vec(),
}); });
@@ -466,6 +499,124 @@ mod tests {
buf buf
} }
/// An internal node laid out exactly as `collect_internal_records` will
/// read it at `depth`: `records` zeroed records, then `children` pointers,
/// all to `child_addr` claiming `child_nrec` records.
fn internal_node(
depth: u16,
node_size: u32,
record_size: u16,
records: usize,
children: usize,
child_addr: u64,
child_nrec: u64,
) -> Vec<u8> {
let max_leaf = max_records_leaf(node_size, record_size);
let nrec_width = bytes_for_max_records(if depth == 1 { max_leaf } else { max_leaf * 2 });
let total_width = if depth > 1 {
bytes_for_max_records(header_max_total_records(max_leaf, depth - 1))
} else {
0
};
let mut buf = b"BTIN".to_vec();
buf.extend_from_slice(&[0, 5]);
buf.resize(buf.len() + records * record_size as usize, 0);
for _ in 0..children {
buf.extend_from_slice(&child_addr.to_le_bytes());
buf.extend_from_slice(&child_nrec.to_le_bytes()[..nrec_width]);
buf.resize(buf.len() + total_width, 0);
}
buf
}
fn header(depth: u16, root: u64, root_nrec: u16, total: u64) -> BTreeV2Header {
BTreeV2Header {
tree_type: 5,
node_size: 512,
record_size: 8,
depth,
root_node_address: root,
num_records_in_root: root_nrec,
total_records: total,
}
}
#[test]
fn a_node_that_is_its_own_child_is_rejected_not_recursed() {
// One internal node whose two children are itself, under a header
// claiming the deepest tree a u16 allows. The layout stops depending
// on depth once the subtree-total width saturates, so every level
// parses cleanly and recursion runs ~65 000 frames deep: before the
// cap this overflowed the stack and aborted the process, from a file
// of under 100 bytes.
let mut data = internal_node(u16::MAX, 512, 8, 1, 2, 0, 1);
data.resize(4096, 0);
let result = collect_btree_v2_records(&data, &header(u16::MAX, 0, 1, 1), 8, 8);
assert!(result.is_err(), "{result:?}");
}
#[test]
fn a_shared_subtree_cannot_multiply_the_work() {
// A chain of distinct levels, each node's children all pointing at the
// single node below, ending in a real leaf. Every node parses and
// nothing is cyclic, yet the leaf is reached fan-out^depth times: 62
// children over 4 levels is ~15 million leaf visits from a few
// kilobytes. A valid tree cannot hold more records than the file has
// room for, so that bounds the traversal instead.
let (node_size, record_size) = (512u32, 8u16);
let fanout = 62usize;
let depth = 4u16;
let leaf = build_leaf_node(5, &[&[0u8; 8][..]]);
// Lay out root first, then each lower level, then the leaf.
let mut nodes: Vec<Vec<u8>> = Vec::new();
let mut addrs = Vec::new();
let mut at = 0u64;
let mut sizes = Vec::new();
for d in (1..=depth).rev() {
let n = internal_node(d, node_size, record_size, fanout - 1, fanout, 0, 0);
sizes.push(n.len());
}
for size in &sizes {
addrs.push(at);
at += *size as u64;
}
let leaf_addr = at;
for (i, d) in (1..=depth).rev().enumerate() {
let (child, child_nrec) = if d == 1 {
(leaf_addr, 1)
} else {
(addrs[i + 1], fanout as u64 - 1)
};
nodes.push(internal_node(
d,
node_size,
record_size,
fanout - 1,
fanout,
child,
child_nrec,
));
}
let mut data: Vec<u8> = nodes.concat();
data.extend_from_slice(&leaf);
data.resize(data.len() + 64, 0);
let started = std::time::Instant::now();
let result =
collect_btree_v2_records(&data, &header(depth, 0, fanout as u16 - 1, u64::MAX), 8, 8);
assert!(
result.is_err(),
"expected a refusal, got {} records",
result.map_or(0, |r| r.len())
);
assert!(
started.elapsed() < std::time::Duration::from_secs(2),
"took {:?}",
started.elapsed()
);
}
#[test] #[test]
fn parse_header() { fn parse_header() {
let data = build_btree_v2_header(5, 512, 11, 0, 0x1000, 3, 3, 8, 8); let data = build_btree_v2_header(5, 512, 11, 0, 0x1000, 3, 3, 8, 8);
@@ -1047,3 +1047,47 @@ with h5py.File("{path_str}", "r") as f:
data[start..start + cols as usize] data[start..start + cols as usize]
); );
} }
#[test]
fn h5py_deep_btree_v2_chunk_index_clawhdf5_reads() {
// Two unlimited dimensions give a B-tree v2 chunk index, and 2x2 chunks
// over 400x400 give 40 000 index records — enough for HDF5 to build a
// tree of depth 2. Small h5py files only ever produce depth-0 trees, so
// this is the one fixture that walks internal nodes: the path where the
// traversal's record budget (the guard against crafted shared-subtree
// trees) is spent, which must never refuse a real file.
skip_if_no_python!();
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("deep_btree.h5");
let path_str = path.display().to_string();
let script = format!(
r#"
import h5py, numpy as np
with h5py.File("{path_str}", "w", libver="latest") as f:
d = f.create_dataset("x", shape=(400, 400), maxshape=(None, None),
chunks=(2, 2), dtype="i4")
d[...] = np.arange(160000, dtype="i4").reshape(400, 400)
"#
);
run_python(&script);
// The fixture is only meaningful if HDF5 really built internal nodes.
let bytes = std::fs::read(&path).unwrap();
let at = bytes
.windows(4)
.position(|w| w == b"BTHD")
.expect("expected a B-tree v2 chunk index");
let depth = u16::from_le_bytes([bytes[at + 12], bytes[at + 13]]);
assert!(
depth >= 1,
"fixture tree has depth {depth}; it tests nothing"
);
let file = File::open(&path).unwrap();
let values = file.dataset("x").unwrap().read_i32().unwrap();
assert_eq!(values.len(), 160_000);
for (i, &v) in values.iter().enumerate() {
assert_eq!(v, i as i32, "element {i}");
}
}
+24
View File
@@ -169,3 +169,27 @@ python3 -m venv .venv && .venv/bin/pip install h5py numpy netCDF4
Set `CLAWHDF5_REQUIRE_INTEROP=1` in any automated runner so a missing Set `CLAWHDF5_REQUIRE_INTEROP=1` in any automated runner so a missing
interpreter is a failure rather than a skip. interpreter is a failure rather than a skip.
---
## Crafted B-tree v2 structures crash or exhaust the reader
**Status:** fixed on `main` (2026-09-20), after v2.6.0. **Every release up to
and including v2.6.0 is affected.**
B-tree v2 traversal (`clawhdf5-format`, `btree_v2::collect_btree_v2_records`)
recursed one frame per level with the depth taken from the file, and followed
child addresses without checking whether they were shared. Two consequences
for anyone reading untrusted files:
- A node that is its own child, under a header claiming 65 535 levels, overflows
the stack and aborts the process. The file is under 100 bytes.
- Levels whose children all point at one node below make the traversal visit it
fan-out^depth times: ~30 million records from ~5 KB, and memory exhaustion one
level deeper.
B-tree v2 backs dense attribute storage, v2 groups, shared object header
messages and chunk indexes, so opening an object that uses any of them is
enough. Both are now errors: depth is capped at 64, and traversal stops once it
has produced more records than the file could physically hold.