diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ae2b02..1949400 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,9 +87,15 @@ header whose continuation chunks chain more than 32 deep (a header that gains a chunk per attribute added when full, as libhdf5 and the editor grow it) was refused with `NestingDepthExceeded`; version-2 headers - stopped at 256 chunks. Chunks are now followed without recursion, in the - same order; a chunk address seen twice (a cycle) or more than 65 536 - chunks are refused. + stopped at 256 chunks. Chunks are now read one at a time from a queue, + in the order their continuation messages are found (libhdf5's + `H5O_protect` order, which the editor already used; a version-1 + chunk's messages used to be inserted at its continuation message), each + buffer released before the next is read; a chunk address seen twice (a + cycle), chunks adding up to more than the file (a crafted chain of + chunks nested in each other made storage with owned buffers read and + hold the square of the file's size), or more than 65 536 chunks are + refused, so a header's chunks read at most the file's size. - Tests: `crates/clawhdf5-tools/tests/edit_coverage_interop.rs` (h5py `earliest`/`v110`/`latest` and clawhdf5-written files; structure comparisons with libhdf5 for version-2 B-trees, shrink on every index, diff --git a/crates/clawhdf5-format/src/object_header.rs b/crates/clawhdf5-format/src/object_header.rs index 383d553..ab652cb 100644 --- a/crates/clawhdf5-format/src/object_header.rs +++ b/crates/clawhdf5-format/src/object_header.rs @@ -232,12 +232,14 @@ impl ObjectHeader { /// end of the chunk, or leftover bytes too few for a message header (a /// "gap", which only version 2 allows). /// - /// Continuation chunks are followed depth-first, as met, with an - /// explicit stack: a chain of continuation chunks as long as libhdf5 - /// writes (each new chunk holding the next continuation message, one - /// per attribute added to a full header) is read without recursion. - /// A chunk address seen twice is a cycle and refused, as is a header of - /// more than [`MAX_V1_CHUNKS`] chunks. + /// Continuation chunks are read in the order their messages are found, + /// as `H5O_protect` loads them (so the messages keep libhdf5's order): + /// a queue of (address, length) pairs, each chunk read, parsed and + /// released before the next, so only one chunk buffer is alive at a + /// time whatever the storage. Every chunk must start at a new address + /// (else a cycle), and the chunks together may be no larger than the + /// file, so the bytes read stay within the file's size; a header of + /// more than [`MAX_V1_CHUNKS`] chunks is refused. fn parse_v1_chunk( file: &S, offset: u64, @@ -246,74 +248,67 @@ impl ObjectHeader { length_size: u8, messages: &mut Vec, ) -> Result { - let mut stack = vec![(read_exact_at(file, offset, length)?, 0usize)]; - let mut seen = BTreeSet::new(); - seen.insert(offset); - let mut count = 0usize; - - while let Some((chunk, pos)) = stack.last_mut() { - let data: &[u8] = chunk; + let mut spans = ChunkSpans::new(file.len(), offset, length)?; + let mut queue: Vec<(u64, usize)> = vec![(offset, length)]; + let mut chunk0_count = 0usize; + let mut next = 0usize; + while let Some(&(chunk_offset, chunk_length)) = queue.get(next) { + let chunk = read_exact_at(file, chunk_offset, chunk_length)?; + let data: &[u8] = &chunk; let end = data.len(); - if *pos >= end { - stack.pop(); - continue; - } - if end - *pos < V1_MSG_HEADER_SIZE { - return Err(FormatError::InvalidObjectHeader( - "gap found in early version of file format", - )); - } - let p = *pos; - let msg_type_raw = LittleEndian::read_u16(&data[p..p + 2]); - let msg_data_size = LittleEndian::read_u16(&data[p + 2..p + 4]) as usize; - let msg_flags = data[p + 4]; - // reserved(3) at p+5..p+8 - let p = p + V1_MSG_HEADER_SIZE; - - if !msg_data_size.is_multiple_of(8) { - return Err(FormatError::InvalidObjectHeader("message not aligned")); - } - if msg_data_size > end - p { - return Err(FormatError::InvalidObjectHeader( - "message size exceeds buffer end", - )); - } - let body = &data[p..p + msg_data_size]; - check_message(1, msg_type_raw, msg_flags, body, offset_size, length_size)?; - let msg_type = MessageType::from_u16(msg_type_raw); - if msg_type != MessageType::Nil { - messages.push(HeaderMessage { - msg_type, - size: msg_data_size, - flags: msg_flags, - creation_order: None, - data: body.to_vec(), - }); - } - // Follow continuations (v1 continuation chunks are just raw - // messages, no signature); check_message has checked the body. - let cont = if msg_type == MessageType::ObjectHeaderContinuation { - Some(( - read_offset(body, 0, offset_size)?, - to_usize(read_offset(body, offset_size as usize, length_size)?)?, - )) - } else { - None - }; - *pos = p + msg_data_size; - // Only the first chunk's messages are held to the prefix count. - if stack.len() == 1 { - count += 1; - } - if let Some((cont_offset, cont_length)) = cont { - if !seen.insert(cont_offset) || seen.len() > MAX_V1_CHUNKS { - return Err(FormatError::NestingDepthExceeded); + let mut pos = 0usize; + let mut count = 0usize; + while pos < end { + if end - pos < V1_MSG_HEADER_SIZE { + return Err(FormatError::InvalidObjectHeader( + "gap found in early version of file format", + )); } - stack.push((read_exact_at(file, cont_offset, cont_length)?, 0)); - } - } + let msg_type_raw = LittleEndian::read_u16(&data[pos..pos + 2]); + let msg_data_size = LittleEndian::read_u16(&data[pos + 2..pos + 4]) as usize; + let msg_flags = data[pos + 4]; + // reserved(3) at pos+5..pos+8 + pos += V1_MSG_HEADER_SIZE; - Ok(count) + if !msg_data_size.is_multiple_of(8) { + return Err(FormatError::InvalidObjectHeader("message not aligned")); + } + if msg_data_size > end - pos { + return Err(FormatError::InvalidObjectHeader( + "message size exceeds buffer end", + )); + } + let body = &data[pos..pos + msg_data_size]; + check_message(1, msg_type_raw, msg_flags, body, offset_size, length_size)?; + count += 1; + let msg_type = MessageType::from_u16(msg_type_raw); + if msg_type != MessageType::Nil { + messages.push(HeaderMessage { + msg_type, + size: msg_data_size, + flags: msg_flags, + creation_order: None, + data: body.to_vec(), + }); + } + // Queue continuations (v1 continuation chunks are just raw + // messages, no signature); check_message has checked the body. + if msg_type == MessageType::ObjectHeaderContinuation { + let cont_offset = read_offset(body, 0, offset_size)?; + let cont_length = + to_usize(read_offset(body, offset_size as usize, length_size)?)?; + spans.add(cont_offset, cont_length)?; + queue.push((cont_offset, cont_length)); + } + pos += msg_data_size; + } + // Only the first chunk's messages are held to the prefix count. + if next == 0 { + chunk0_count = count; + } + next += 1; + } + Ok(chunk0_count) } fn parse_v2( @@ -436,15 +431,14 @@ impl ObjectHeader { &mut continuations, )?; - // Follow continuations. A chunk address seen twice is a cycle in - // malformed data; a valid header can have many chunks (libhdf5 adds - // one whenever a message no longer fits), up to the same bound as a + // Follow continuations, one chunk buffer at a time. A chunk address + // seen twice is a cycle in malformed data, and the chunks may add up + // to no more than the file; a valid header can have many chunks (libhdf5 adds one + // whenever a message no longer fits), up to the same bound as a // version-1 header. - let mut seen = BTreeSet::new(); + let mut spans = ChunkSpans::new(file.len(), base as u64, chunk0_msg_end.saturating_add(4))?; while let Some((cont_offset, cont_length)) = continuations.pop() { - if !seen.insert(cont_offset) || seen.len() > MAX_V1_CHUNKS { - return Err(FormatError::NestingDepthExceeded); - } + spans.add(cont_offset as u64, cont_length)?; Self::parse_v2_continuation( file, cont_offset as u64, @@ -607,6 +601,45 @@ const V2_PREFIX_MAX: usize = 34; /// Size of a version-1 message header: type(2) + size(2) + flags(1) + reserved(3). const V1_MSG_HEADER_SIZE: usize = 8; +/// The chunks of one object header read so far. A chunk starting where +/// another did is a cycle. Chunks of a valid header do not overlap, so +/// together they are no larger than the file; a header whose chunks add up +/// to more is refused, which bounds what its chunks can make a reader read +/// (a crafted chain of chunks each nested in the last would otherwise read +/// the file over and over). Overlap itself is not refused: libhdf5 reads +/// such headers (`cve-2025-7067.h5` has one). +struct ChunkSpans { + starts: BTreeSet, + /// Bytes of the chunks so far, and the most they may add up to. + total: u64, + budget: u64, +} + +impl ChunkSpans { + fn new(file_len: u64, start: u64, len: usize) -> Result { + let mut s = Self { + starts: BTreeSet::new(), + total: 0, + budget: file_len, + }; + s.add(start, len)?; + Ok(s) + } + + fn add(&mut self, start: u64, len: usize) -> Result<(), FormatError> { + if !self.starts.insert(start) || self.starts.len() > MAX_V1_CHUNKS { + return Err(FormatError::NestingDepthExceeded); + } + self.total = self.total.saturating_add(len as u64); + if self.total > self.budget { + return Err(FormatError::InvalidObjectHeader( + "object header chunks larger than the file", + )); + } + Ok(()) + } +} + /// Most chunks a version-1 object header may have (malformed-data guard; /// libhdf5 has no limit, and a header that gains one continuation chunk per /// attribute added can have many). @@ -972,6 +1005,104 @@ mod tests { assert_eq!(spaces, (0..200).map(|k| k as u8).collect::>()); } + /// A crafted version-1 header whose continuation chunks nest: each + /// chunk's continuation message points at the rest of that chunk. Read + /// depth-first with every enclosing chunk kept alive, from storage that + /// hands out owned buffers, it read n^2 bytes and held them all at once + /// (a 192 KB file read 768 MB). Chunks adding up to more than the file + /// are refused, and the bytes read stay within the file's size. + #[test] + fn nested_v1_continuation_chunks_are_bounded() { + use crate::storage::CountingStorage; + let n = 2000u64; + let a = 64u64; + let cont = |addr: u64, len: u64| { + let mut m = vec![0x10, 0, 16, 0, 0, 0, 0, 0]; + m.extend_from_slice(&addr.to_le_bytes()); + m.extend_from_slice(&len.to_le_bytes()); + m + }; + // Prefix: version 1, one message, reference count 1, 24 bytes. + let mut buf = vec![1, 0, 1, 0, 1, 0, 0, 0, 24, 0, 0, 0, 0, 0, 0, 0]; + buf.extend_from_slice(&cont(a, 24 * n)); + buf.resize(a as usize, 0); + for k in 0..n { + if k + 1 < n { + buf.extend_from_slice(&cont(a + 24 * (k + 1), 24 * (n - k - 1))); + } else { + buf.extend_from_slice(&[0, 0, 16, 0, 0, 0, 0, 0]); + buf.extend_from_slice(&[0; 16]); + } + } + let len = buf.len() as u64; + let s = CountingStorage::new(buf); + assert!(matches!( + ObjectHeader::parse_in(&s, 0, 8, 8), + Err(FormatError::InvalidObjectHeader( + "object header chunks larger than the file" + )) + )); + assert!( + s.bytes_read() <= 2 * len, + "read {} of {len}", + s.bytes_read() + ); + } + + /// libhdf5 reads a continuation chunk that overlaps the chunk holding + /// its message (`cve-2025-7067.h5` has one), and so does this reader. + #[test] + fn overlapping_v1_continuation_chunk_is_read() { + // Chunk 0 (at 16): continuation (24 bytes), then a NIL message at + // 40; the continuation chunk is that NIL message's 8-byte header. + let mut cont = 40u64.to_le_bytes().to_vec(); + cont.extend_from_slice(&8u64.to_le_bytes()); + let data = build_v1_header(&[(0x0010, &cont[..], 0), (0x0000, &[][..], 0)], 8, 8); + let hdr = ObjectHeader::parse(&data, 0, 8, 8).unwrap(); + assert_eq!(hdr.messages.len(), 1); + } + + /// A valid chain over owned-buffer storage reads each chunk once. + #[test] + fn long_v1_chain_reads_each_chunk_once() { + use crate::storage::CountingStorage; + let data = v1_chain(3000, false); + let len = data.len() as u64; + let s = CountingStorage::new(data); + let hdr = ObjectHeader::parse_in(&s, 0, 8, 8).unwrap(); + assert_eq!( + hdr.messages + .iter() + .filter(|m| m.msg_type == MessageType::Dataspace) + .count(), + 3000 + ); + assert!(s.bytes_read() <= len, "read {} of {len}", s.bytes_read()); + } + + /// Continuation chunks are read in the order their messages are found + /// (libhdf5's `H5O_protect`), so a chunk's messages follow every + /// message of the chunk before, not the continuation message. + #[test] + fn v1_continuation_messages_keep_libhdf5_order() { + // Chunk 0: continuation to A, dataspace [1]; A: dataspace [2]. + let a = 64u64; + let mut cont = a.to_le_bytes().to_vec(); + cont.extend_from_slice(&16u64.to_le_bytes()); + let mut data = build_v1_header(&[(0x0010, &cont[..], 0), (0x0001, &[1; 8][..], 0)], 8, 8); + data.resize(a as usize, 0); + data.extend_from_slice(&[1, 0, 8, 0, 0, 0, 0, 0]); + data.extend_from_slice(&[2; 8]); + let hdr = ObjectHeader::parse(&data, 0, 8, 8).unwrap(); + let spaces: Vec = hdr + .messages + .iter() + .filter(|m| m.msg_type == MessageType::Dataspace) + .map(|m| m.data[0]) + .collect(); + assert_eq!(spaces, [1, 2]); + } + #[test] fn v1_continuation_cycles_are_refused() { let data = v1_chain(5, true);