diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fc429d..75b1ba3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -197,6 +197,13 @@ takes `--f32`; it had kept printing "f32" after the default changed. ### Interop +- **h5py could not open chunked datasets we wrote with a chunk dimension + from 65 536 to 16 777 215.** A version-4 layout must store its chunk + dimensions in the fewest bytes that hold the largest (3 for 70 000); + the writer rounded 3 up to 4, and HDF5 2.0.0 (h5py 3.16) refuses that + ("stored chunk dimension encoding length does not match value calculated + from chunk dimensions"). Newer libhdf5 and clawhdf5 read those files; new + files use the exact width. Test: `we_write_chunk_dimensions_in_the_fewest_bytes`. - **Conformance sweep in the repo** (`conformance/`, report in `CONFORMANCE.md`). `conformance/run.sh` fetches eight public HDF5 corpora pinned by commit (libhdf5's test files, the HDF Group's CVE reproducers, @@ -296,6 +303,84 @@ - CI keeps zlib-ng building and tested; the arm64 job no longer needs cmake. ### Correctness +- **Corrupt files libhdf5 refuses are now refused instead of read.** On the + HDF Group's CVE reproducers, 18 objects that libhdf5 (HDF5 2.0, through + h5py) refuses to open were read by clawhdf5, some as wrong data (a chunk + dimension of 0 read as all fill values; chunks read at offsets off the + chunk grid). The + parser now makes libhdf5's checks, with libhdf5's error text: + - object headers (`FormatError::InvalidObjectHeader`): every message of a + v1 chunk is read and more than the prefix's count is refused (the rest + used to be dropped); v1 message sizes must be multiples of 8 and a v1 + chunk cannot end in a gap; a message running past its chunk is an error + (it used to end the chunk quietly); contradictory message flags; a + message of a class that cannot be shared flagged shareable; a + reference-count message in a v1 header; malformed continuation, + reference-count and modification-time messages; unknown v2 header + flags. + - datatypes (`FormatError::InvalidDatatype`): size 0; integer bits outside + the type; float exponent/mantissa outside the type, empty or + overlapping; a compound with no members, a member outside the compound, + a duplicate name or overlapping members; an enum whose size differs from + its base type's or with an empty name; array rank over 32 or a zero + dimension; an opaque tag length that is not a multiple of 8; in a + version-1 (unchecksummed) header, a numeric type that leaves more than + half its bits unused (`Datatype::parse_in_header`, + `Datatype::check_unused_bits`). A v1/v2 float's class bit 6 was read as + VAX byte order; libhdf5 ignores it before version 3, and so does this. + The overlap check measures each earlier member by its stored size, as + libhdf5 does, so a variable-length member (4 + offset size + 4 bytes) + in a file with 4-byte offsets does not overlap the member after it. + - chunked layouts (`FormatError::InvalidChunkDimensions`): a zero chunk + dimension, a chunk rank that does not match the dataspace, a chunk of + 4 GiB or more indexed by a v1 B-tree (layout version 3 or earlier; + 0x80000000-sized chunks hung the reader — layout versions 4 and 5 allow + larger chunks, and HDF5 2.0 writes them), an element size in the + layout that differs from the datatype's stored size (the chunks were + laid out with the wrong element size), and v1 B-tree + chunk keys whose offsets are not multiples of the chunk dimensions, + including the keys that only bound a node + (`chunked_read::collect_chunk_info_checked`). + - truncated files (`FormatError::TruncatedFile`, `Superblock::data_end`): + a file shorter than the end of file its superblock records is refused + ("truncated file"), and nothing past that end is read. Every reader + does this: `File`, `LazyFile` and `MmapFile`, and in `clawhdf5-io` + `NativeVol` (at `open`, and on read for `from_bytes`), + `AsyncHDF5File` and `MpiVol` (the MPI path is not built in CI: it + needs an MPI installation). + - the writer: `FileWriter::finish()` / `FileBuilder::finish()` refuse a + datatype the reader would refuse (`FormatError::SerializationError`, + "datatype cannot be written: ..."), such as a compound with a repeated + field name or no fields, or an enum member with an empty name + (`CompoundTypeBuilder` and `EnumTypeBuilder` build them without + complaint). These were never valid HDF5 — h5py refuses them — and + clawhdf5 wrote them, which made files it could not read back. + + Checks newer libhdf5 releases make but HDF5 2.0 does not (bit-field + offsets, the variable-length kind, array sizes) are left out, so files + h5py opens still open. Two libhdf5 checks are skipped on purpose because + clawhdf5 up to v2.7.0 wrote files that fail them without being wrong: + the sign bit of every float at position 63, and a size-0 string type for + an empty-string attribute (new fixtures written by v2.7.0 guard this). + Conformance: 569 -> 571 ok (h5stat_err_refcount.h5, + h5clear_fsm_persist_less.h5), and 17 of the 18 CVE objects now fail as in + libhdf5 (see `docs/known-issues.md` for the one left), as do 10 files + h5py refuses as truncated. Tests: + `header_validation_interop.rs` (h5py writes, the test damages a copy, both + libraries must refuse it), `legacy_writer_files.rs`, and unit tests next + to each check. **Breaking (format crate):** `FormatError` gained + `InvalidObjectHeader`, `InvalidDatatype`, `InvalidChunkDimensions` and + `TruncatedFile`; an exhaustive `match` on it needs the new arms. +- **Chunked datasets whose chunk dimensions take 3, 5, 6 or 7 bytes did not + open.** A version-4 layout (`libver="latest"`) stores each chunk dimension + in the fewest bytes that hold the largest one, so a chunk dimension from + 65 536 to 16 777 215 (e.g. h5py `chunks=(70000,)`) takes 3 bytes; only 1, 2, + 4 and 8 were read, and the rest failed with `UnexpectedEof`. Widths 1-8 are + read now, and 0 or more than 8 is refused as libhdf5 refuses it. A width + larger than needed is accepted: HDF5 2.0.0 refuses one ("stored chunk + dimension encoding length does not match"), but libhdf5 since + HDFGroup/hdf5@e124c36 (2026-06-05) reads it, and clawhdf5 itself wrote such + layouts. - `clawhdf5-format` VDS: variable-length and reference data from a source in another file is refused. Those elements are global-heap IDs and object addresses in the source file; copied into the virtual dataset they would diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 1298cf0..87c660d 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -309,11 +309,19 @@ impl<'a> Ctx<'a> { } } + fn read_named_datatype(&self, h: &ObjectHeader) -> Result<(), String> { + let dtb = self + .payload(h, MessageType::Datatype)? + .ok_or("MissingMessage(Datatype)")?; + Datatype::parse_in_header(&dtb, h.version).map_err(e)?; + Ok(()) + } + fn read_dataset(&self, h: &ObjectHeader, rec: &mut Map) -> Result<(), String> { let dtb = self .payload(h, MessageType::Datatype)? .ok_or("MissingMessage(Datatype)")?; - let (dt, _) = Datatype::parse(&dtb).map_err(e)?; + let (dt, _) = Datatype::parse_in_header(&dtb, h.version).map_err(e)?; rec.insert("dtype".into(), Value::String(dtype_str(&dt))); let dsb = self .payload(h, MessageType::Dataspace)? @@ -716,6 +724,17 @@ fn main() { return; } }; + // libhdf5 refuses a truncated file and reads nothing past the recorded + // end of file. + let base = (data.len() - hdf5.len()) as u64; + let hdf5 = match sb.data_end(base, data.len() as u64) { + Ok(end) => &hdf5[..end as usize], + Err(err) => { + top.insert("open_error".into(), Value::String(e(err))); + println!("{}", Value::Object(top)); + return; + } + }; top.insert("superblock_version".into(), json!(sb.version)); let ctx = Ctx { data: hdf5, @@ -778,6 +797,13 @@ fn main() { { rec.insert("error".into(), Value::String(msg)); } + // Opening a committed datatype decodes it (h5py's `f[name]` fails on + // one libhdf5 cannot decode), so decode it here too. + if kind == "datatype" + && let Err(msg) = guarded(|| ctx.read_named_datatype(&h)) + { + rec.insert("error".into(), Value::String(msg)); + } if kind != "datatype" { match guarded(|| ctx.attrs(&h)) { Ok(m) => { diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index c844c3c..11bde3e 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -362,6 +362,18 @@ fn extract_name(bytes: &[u8]) -> String { String::from_utf8_lossy(&bytes[..end]).into_owned() } +/// An attribute's datatype gets libhdf5's extra check for a header without +/// a checksum (see [`Datatype::check_unused_bits`]). +fn check_in_header( + attr: AttributeMessage, + header: &ObjectHeader, +) -> Result { + if header.version == 1 { + attr.datatype.check_unused_bits()?; + } + Ok(attr) +} + /// Extract all attribute messages from an object header. pub fn extract_attributes( header: &ObjectHeader, @@ -371,7 +383,7 @@ pub fn extract_attributes( for msg in &header.messages { if msg.msg_type == MessageType::Attribute { let attr = AttributeMessage::parse(&msg.data, length_size)?; - attrs.push(attr); + attrs.push(check_in_header(attr, header)?); } } Ok(attrs) @@ -465,6 +477,7 @@ fn extract_attributes_with( } else { AttributeMessage::parse_in_file(&msg.data, file_data, offset_size, length_size) }; + let attr = attr.and_then(|a| check_in_header(a, header)); match attr { Ok(attr) => attrs.push(attr), Err(e) => on_error(e)?, @@ -573,7 +586,8 @@ mod tests { /// Build an f64 LE datatype message. fn build_f64_dt() -> Vec { - let mut buf = build_dt_header(1, 1, [0x00, 0x00, 0x02], 8); + // Sign bit 63 (bits 8-15 of the class bits). + let mut buf = build_dt_header(1, 1, [0x20, 63, 0x00], 8); let mut props = [0u8; 12]; props[2..4].copy_from_slice(&64u16.to_le_bytes()); // bit_precision props[4] = 52; // exp_location diff --git a/crates/clawhdf5-format/src/chunked_read.rs b/crates/clawhdf5-format/src/chunked_read.rs index 3a3dc05..afff80d 100644 --- a/crates/clawhdf5-format/src/chunked_read.rs +++ b/crates/clawhdf5-format/src/chunked_read.rs @@ -148,6 +148,96 @@ pub(crate) fn checked_byte_len(elements: u64, elem_size: usize) -> Result Result<(usize, Vec), FormatError> { + let rank = chunk_dimensions.len().checked_sub(1).ok_or_else(|| { + FormatError::InvalidChunkDimensions("chunked layout has no dimensions".into()) + })?; + if dataspace.dimensions.len() != rank { + return Err(FormatError::InvalidChunkDimensions(format!( + "dimensionality of chunks doesn't match the dataspace (chunk rank {rank}, \ + dataspace rank {})", + dataspace.dimensions.len() + ))); + } + let spatial = &chunk_dimensions[..rank]; + if let Some(d) = spatial.iter().position(|&c| c == 0) { + return Err(FormatError::InvalidChunkDimensions(format!( + "chunk size must be > 0, dim = {d}" + ))); + } + let bytes = spatial + .iter() + .fold(elem_size as u128, |acc, &c| acc * u128::from(c)); + if layout_version < 4 && bytes > u128::from(u32::MAX) { + return Err(FormatError::InvalidChunkDimensions(format!( + "chunk size must be < 4GB with v1 b-tree index (chunk {spatial:?} of {elem_size}-byte elements)" + ))); + } + Ok((rank, spatial.iter().map(|&c| c as usize).collect())) +} + +/// The size of one element of `dt` as stored in the file: a +/// variable-length element is its length (4), a global heap address +/// (`offset_size`) and an index (4), not the 16 of [`Datatype::type_size`]. +fn stored_element_size(dt: &Datatype, offset_size: u8) -> u64 { + match dt { + Datatype::VariableLength { .. } => 8 + u64::from(offset_size), + Datatype::Array { + base_type, + dimensions, + } => dimensions + .iter() + .fold(stored_element_size(base_type, offset_size), |acc, &d| { + acc.saturating_mul(u64::from(d)) + }), + _ => u64::from(dt.type_size()), + } +} + +/// A chunked layout records the element size as its last dimension, and +/// libhdf5 refuses a dataset whose datatype has another size +/// (`H5D__chunk_set_sizes`: "stored datatype size in chunk layout does not +/// match datatype description"). Reading it anyway laid the chunks out with +/// the wrong element size. +pub(crate) fn check_chunk_element_size( + layout: &DataLayout, + datatype: &Datatype, + offset_size: u8, +) -> Result<(), FormatError> { + let DataLayout::Chunked { + chunk_dimensions, .. + } = layout + else { + return Ok(()); + }; + let Some(&stored) = chunk_dimensions.last() else { + return Ok(()); + }; + let expected = stored_element_size(datatype, offset_size); + if u64::from(stored) != expected { + return Err(FormatError::InvalidChunkDimensions(format!( + "stored datatype size in chunk layout does not match datatype description \ + (layout {stored} bytes, datatype {expected})" + ))); + } + Ok(()) +} + /// Product of chunk dimensions times the element size, overflow-checked. pub(crate) fn checked_chunk_byte_len( chunk_dims: &[usize], @@ -222,7 +312,76 @@ pub fn collect_chunk_info( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - collect_chunk_info_inner(file_data, btree_address, ndims, offset_size, length_size, 0) + collect_chunk_info_inner( + file_data, + btree_address, + ndims, + None, + offset_size, + length_size, + 0, + ) +} + +/// [`collect_chunk_info`] for a layout with these `chunk_dimensions` (the +/// layout message's list, element size last), checking every key of the +/// B-tree as libhdf5 does (`H5D__btree_decode_key`): each coordinate offset +/// must be a multiple of its chunk dimension. That includes the keys that +/// only bound a node (internal-node keys and each node's final key), which +/// is where a corrupt chunk dimension shows when the chunks themselves all +/// start at offset 0 in that dimension (`cve-2018-11205`). A key that fails +/// ("bad coordinate offset") means a corrupt index or chunk dimension; the +/// chunks were read at the wrong place, or the dataset read as fill values. +pub fn collect_chunk_info_checked( + file_data: &[u8], + btree_address: u64, + chunk_dimensions: &[u32], + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + collect_chunk_info_inner( + file_data, + btree_address, + chunk_dimensions.len(), + Some(chunk_dimensions), + offset_size, + length_size, + 0, + ) +} + +/// Check one v1 B-tree chunk key's offsets (see +/// [`collect_chunk_info_checked`]). +fn check_key_offsets(offsets: &[u64], chunk_dimensions: &[u32]) -> Result<(), FormatError> { + for (&offset, &dim) in offsets.iter().zip(chunk_dimensions) { + if dim == 0 || offset % u64::from(dim) != 0 { + return Err(FormatError::ChunkedReadError(format!( + "bad coordinate offset {offsets:?} for chunk dimensions {chunk_dimensions:?}" + ))); + } + } + Ok(()) +} + +/// Read the `ndims` 8-byte offsets of the chunk key at `pos` (after its +/// chunk size and filter mask) and check them when `chunk_dimensions` is +/// given. +fn read_key_offsets( + file_data: &[u8], + pos: usize, + ndims: usize, + chunk_dimensions: Option<&[u32]>, +) -> Result, FormatError> { + let mut offsets = Vec::with_capacity(ndims); + let mut kp = pos + 8; + for _ in 0..ndims { + offsets.push(read_offset(file_data, kp, CHUNK_KEY_OFFSET_SIZE)?); + kp += CHUNK_KEY_OFFSET_SIZE as usize; + } + if let Some(dims) = chunk_dimensions { + check_key_offsets(&offsets, dims)?; + } + Ok(offsets) } /// Width of each chunk offset in a v1 chunk B-tree key, independent of the @@ -237,6 +396,7 @@ fn collect_chunk_info_inner( file_data: &[u8], btree_address: u64, ndims: usize, + chunk_dimensions: Option<&[u32]>, offset_size: u8, _length_size: u8, depth: usize, @@ -296,12 +456,7 @@ fn collect_chunk_info_inner( file_data[pos + 6], file_data[pos + 7], ]); - let mut offsets = Vec::with_capacity(ndims); - let mut kp = pos + 8; - for _ in 0..ndims { - offsets.push(read_offset(file_data, kp, CHUNK_KEY_OFFSET_SIZE)?); - kp += CHUNK_KEY_OFFSET_SIZE as usize; - } + let offsets = read_key_offsets(file_data, pos, ndims, chunk_dimensions)?; pos += key_size; // Parse child address @@ -315,7 +470,8 @@ fn collect_chunk_info_inner( address, }); } - // Skip final key + // The final key only bounds the node; libhdf5 still checks it. + read_key_offsets(file_data, pos, ndims, chunk_dimensions)?; Ok(chunks) } else { // Internal node: recurse into children @@ -324,11 +480,13 @@ fn collect_chunk_info_inner( let mut child_addrs = Vec::with_capacity(entries_used); for _ in 0..entries_used { - pos += key_size; // skip key + read_key_offsets(file_data, pos, ndims, chunk_dimensions)?; + pos += key_size; let child_addr = read_offset(file_data, pos, offset_size)?; child_addrs.push(child_addr); pos += os; } + read_key_offsets(file_data, pos, ndims, chunk_dimensions)?; let mut all_chunks = Vec::new(); for child_addr in child_addrs { @@ -336,6 +494,7 @@ fn collect_chunk_info_inner( file_data, child_addr, ndims, + chunk_dimensions, offset_size, _length_size, depth + 1, @@ -549,30 +708,13 @@ pub fn list_chunks( .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; // Both v3 and v4 include element size as last dim (rank+1) - let ndims = chunk_dimensions.len(); - let rank = ndims - .checked_sub(1) - .ok_or_else(|| FormatError::ChunkedReadError("chunked layout has no dimensions".into()))?; - let chunk_dims: Vec = chunk_dimensions[..rank] - .iter() - .map(|&d| d as usize) - .collect(); - + let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); - if ds_dims.len() != rank { - return Err(FormatError::ChunkedReadError(format!( - "rank mismatch: dataspace has {} dims, layout has {} chunk dims (rank={})", - ds_dims.len(), - chunk_dimensions.len(), - rank - ))); - } // Collect chunks based on version and index type let mut chunks = match (version, chunk_index_type) { (3, _) => { - let ndims = chunk_dimensions.len(); // rank+1 - collect_chunk_info(file_data, addr, ndims, offset_size, length_size)? + collect_chunk_info_checked(file_data, addr, chunk_dimensions, offset_size, length_size)? } (4, Some(1)) => { // Single chunk — one chunk covering the entire dataset @@ -679,6 +821,7 @@ pub fn read_chunked_data( offset_size: u8, length_size: u8, ) -> Result, FormatError> { + check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; let (chunks, chunk_dims) = list_chunks( file_data, @@ -802,12 +945,13 @@ pub fn read_chunked_data_cached( length_size: u8, cache: &ChunkCache, ) -> Result, FormatError> { - let (chunk_dimensions, addr_opt) = match layout { + let (chunk_dimensions, version, addr_opt) = match layout { DataLayout::Chunked { chunk_dimensions, + version, btree_address, .. - } => (chunk_dimensions, *btree_address), + } => (chunk_dimensions, *version, *btree_address), _ => { return Err(FormatError::ChunkedReadError( "expected chunked layout".into(), @@ -818,25 +962,10 @@ pub fn read_chunked_data_cached( let addr = addr_opt .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; + check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; - let ndims = chunk_dimensions.len(); - let rank = ndims - .checked_sub(1) - .ok_or_else(|| FormatError::ChunkedReadError("chunked layout has no dimensions".into()))?; - let chunk_dims: Vec = chunk_dimensions[..rank] - .iter() - .map(|&d| d as usize) - .collect(); - + let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); - if ds_dims.len() != rank { - return Err(FormatError::ChunkedReadError(format!( - "rank mismatch: dataspace has {} dims, layout has {} chunk dims (rank={})", - ds_dims.len(), - chunk_dimensions.len(), - rank - ))); - } // The per-file cache is shared across datasets (and threads); every // lookup is keyed by this dataset's chunk-index address, so another @@ -1123,12 +1252,13 @@ pub fn read_chunked_data_sweep( cache: &ChunkCache, sweep: &mut SweepContext, ) -> Result, FormatError> { - let (chunk_dimensions, addr_opt) = match layout { + let (chunk_dimensions, version, addr_opt) = match layout { DataLayout::Chunked { chunk_dimensions, + version, btree_address, .. - } => (chunk_dimensions, *btree_address), + } => (chunk_dimensions, *version, *btree_address), _ => { return Err(FormatError::ChunkedReadError( "expected chunked layout".into(), @@ -1139,25 +1269,10 @@ pub fn read_chunked_data_sweep( let addr = addr_opt .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; + check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; - let ndims = chunk_dimensions.len(); - let rank = ndims - .checked_sub(1) - .ok_or_else(|| FormatError::ChunkedReadError("chunked layout has no dimensions".into()))?; - let chunk_dims: Vec = chunk_dimensions[..rank] - .iter() - .map(|&d| d as usize) - .collect(); - + let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); - if ds_dims.len() != rank { - return Err(FormatError::ChunkedReadError(format!( - "rank mismatch: dataspace has {} dims, layout has {} chunk dims (rank={})", - ds_dims.len(), - chunk_dimensions.len(), - rank - ))); - } // The per-file cache is shared across datasets (and threads); every // lookup is keyed by this dataset's chunk-index address, so another @@ -1275,12 +1390,13 @@ pub fn read_chunked_data_indexed( length_size: u8, cache: &ChunkCache, ) -> Result, FormatError> { - let (chunk_dimensions, addr_opt) = match layout { + let (chunk_dimensions, version, addr_opt) = match layout { DataLayout::Chunked { chunk_dimensions, + version, btree_address, .. - } => (chunk_dimensions, *btree_address), + } => (chunk_dimensions, *version, *btree_address), _ => { return Err(FormatError::ChunkedReadError( "expected chunked layout".into(), @@ -1291,25 +1407,10 @@ pub fn read_chunked_data_indexed( let addr = addr_opt .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; + check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; - let ndims = chunk_dimensions.len(); - let rank = ndims - .checked_sub(1) - .ok_or_else(|| FormatError::ChunkedReadError("chunked layout has no dimensions".into()))?; - let chunk_dims: Vec = chunk_dimensions[..rank] - .iter() - .map(|&d| d as usize) - .collect(); - + let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); - if ds_dims.len() != rank { - return Err(FormatError::ChunkedReadError(format!( - "rank mismatch: dataspace has {} dims, layout has {} chunk dims (rank={})", - ds_dims.len(), - chunk_dimensions.len(), - rank - ))); - } // Chunk index and assembly plan for this dataset, built on first access // and kept per dataset (keyed by chunk-index address) in the shared cache. @@ -1620,11 +1721,12 @@ mod tests { write_offset(&mut buf, chunk.address, offset_size); } - // Final key (dummy) + // Final key (its offsets must be on the chunk grid, as libhdf5 + // checks; 0 always is) buf.extend_from_slice(&0u32.to_le_bytes()); // chunk_size buf.extend_from_slice(&0u32.to_le_bytes()); // filter_mask for _ in 0..ndims { - write_offset(&mut buf, u64::MAX, 8); + write_offset(&mut buf, 0, 8); } buf @@ -1632,6 +1734,48 @@ mod tests { // --- ChunkInfo collection tests --- + #[test] + fn checked_collection_refuses_keys_off_the_chunk_grid() { + let chunk = |offsets: Vec, address| ChunkInfo { + chunk_size: 80, + filter_mask: 0, + offsets, + address, + }; + let good = + build_chunk_btree_leaf(&[chunk(vec![0, 0], 0x100), chunk(vec![10, 0], 0x200)], 2, 8); + assert_eq!( + collect_chunk_info_checked(&good, 0, &[10, 8], 8, 8) + .unwrap() + .len(), + 2 + ); + // A chunk key off the grid. + let bad = + build_chunk_btree_leaf(&[chunk(vec![0, 0], 0x100), chunk(vec![7, 0], 0x200)], 2, 8); + assert!(collect_chunk_info(&bad, 0, 2, 8, 8).is_ok()); + assert!(matches!( + collect_chunk_info_checked(&bad, 0, &[10, 8], 8, 8), + Err(FormatError::ChunkedReadError(m)) if m.starts_with("bad coordinate offset") + )); + // cve-2018-11205: the chunks all start at 0 in dimension 1, and only + // the node's final key shows the chunk dimension is wrong. + let mut two_d = build_chunk_btree_leaf( + &[chunk(vec![0, 0, 0], 0x100), chunk(vec![10, 0, 0], 0x200)], + 3, + 8, + ); + // Final key: (20, 20, 0), the end of a 20 x 20 dataset. + let final_key = two_d.len() - 24; + two_d[final_key..final_key + 8].copy_from_slice(&20u64.to_le_bytes()); + two_d[final_key + 8..final_key + 16].copy_from_slice(&20u64.to_le_bytes()); + assert!(collect_chunk_info_checked(&two_d, 0, &[10, 20, 4], 8, 8).is_ok()); + assert!(matches!( + collect_chunk_info_checked(&two_d, 0, &[10, 32788, 4], 8, 8), + Err(FormatError::ChunkedReadError(m)) if m.starts_with("bad coordinate offset [20, 20, 0]") + )); + } + #[test] fn collect_two_chunks_from_leaf() { let ndims = 2; // rank+1 for 1D dataset @@ -1750,6 +1894,51 @@ mod tests { use crate::dataspace::{Dataspace, DataspaceType}; use crate::datatype::{Datatype, DatatypeByteOrder}; + #[test] + fn chunk_geometry_matches_libhdf5_open_checks() { + let space = |dims: &[u64]| Dataspace { + space_type: DataspaceType::Simple, + rank: dims.len() as u8, + dimensions: dims.to_vec(), + max_dimensions: None, + }; + for v in [3, 4] { + assert_eq!( + chunk_geometry(&[4, 5, 8], v, &space(&[10, 10]), 8).unwrap(), + (2, vec![4, 5]) + ); + // Rank mismatch. + assert!(matches!( + chunk_geometry(&[4, 8], v, &space(&[10, 10]), 8), + Err(FormatError::InvalidChunkDimensions(m)) if m.contains("doesn't match") + )); + // Zero dimension (a layout built in memory, bypassing the parser). + assert!(matches!( + chunk_geometry(&[4, 0, 8], v, &space(&[10, 10]), 8), + Err(FormatError::InvalidChunkDimensions(m)) if m.contains("must be > 0") + )); + assert!(chunk_geometry(&[0xFFFF_FFFF, 1], v, &space(&[10]), 1).is_ok()); + } + // With a v1 B-tree index (layout version 3) the largest chunk is + // 4 GiB - 1 bytes: 0x80000000 x 4-byte elements (8 GiB) is refused. + // These dims used to hang the reader. + assert!(matches!( + chunk_geometry(&[0x8000_0000, 4], 3, &space(&[10]), 4), + Err(FormatError::InvalidChunkDimensions(m)) if m.contains("4GB with v1 b-tree") + )); + assert!(matches!( + chunk_geometry(&[0xFFFF_FFFF, 0xFFFF_FFFF, 1], 3, &space(&[10, 10]), 1), + Err(FormatError::InvalidChunkDimensions(m)) if m.contains("4GB with v1 b-tree") + )); + // The other chunk indexes (layout version 4, and 5, which is read as + // 4) allow chunks of 4 GiB and more; HDF5 2.0 writes them. + assert_eq!( + chunk_geometry(&[0x2000_0001, 8], 4, &space(&[10]), 8).unwrap(), + (1, vec![0x2000_0001]) + ); + assert!(chunk_geometry(&[0xFFFF_FFFF, 0xFFFF_FFFF, 1], 4, &space(&[10, 10]), 1).is_ok()); + } + fn make_f64_type() -> Datatype { Datatype::FloatingPoint { size: 8, @@ -1863,8 +2052,8 @@ mod tests { 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:?}" + matches!(result, Err(FormatError::InvalidChunkDimensions(_))), + "expected a clean InvalidChunkDimensions, got {result:?}" ); } diff --git a/crates/clawhdf5-format/src/chunked_write.rs b/crates/clawhdf5-format/src/chunked_write.rs index ff5081e..dcd0e43 100644 --- a/crates/clawhdf5-format/src/chunked_write.rs +++ b/crates/clawhdf5-format/src/chunked_write.rs @@ -383,39 +383,7 @@ fn serialize_v4_single_chunk( let ndims = chunk_dims.len() as u8 + 1; buf.push(ndims); - // dim_size_encoded_length: how many bytes per dimension - // We need to figure out the minimum encoding width - let max_dim = chunk_dims - .iter() - .map(|&d| d as u64) - .chain(core::iter::once(element_size as u64)) - .max() - .unwrap_or(1); - let dim_encoded_len: u8 = if max_dim <= 0xFF { - 1 - } else if max_dim <= 0xFFFF { - 2 - } else { - 4 - }; - buf.push(dim_encoded_len); - - // dimension sizes (chunk dims + element size) - for &d in chunk_dims { - match dim_encoded_len { - 1 => buf.push(d as u8), - 2 => buf.extend_from_slice(&(d as u16).to_le_bytes()), - 4 => buf.extend_from_slice(&d.to_le_bytes()), - _ => {} - } - } - // Element size dimension - match dim_encoded_len { - 1 => buf.push(element_size as u8), - 2 => buf.extend_from_slice(&(element_size as u16).to_le_bytes()), - 4 => buf.extend_from_slice(&element_size.to_le_bytes()), - _ => {} - } + push_v4_chunk_dims(&mut buf, chunk_dims, element_size); // chunk index type = 1 (single chunk) buf.push(1); @@ -465,6 +433,25 @@ fn serialize_v4_fixed_array( /// The part of a v4 chunked layout message before the chunk index type: /// version, class, flags and the chunk dimensions (plus the element size). +/// Append a v4 layout's dimension width and its dimensions (the chunk +/// dimensions, then the element size). Each takes the fewest bytes that hold +/// the largest, as libhdf5 computes it (`H5D__chunk_set_sizes`: +/// `(log2(dim) + 8) / 8`); HDF5 2.0.0 refuses any other width. +pub(crate) fn push_v4_chunk_dims(buf: &mut Vec, chunk_dims: &[u32], element_size: u32) { + let max_dim = chunk_dims + .iter() + .copied() + .chain(core::iter::once(element_size)) + .max() + .unwrap_or(1) + .max(1); + let width = (32 - max_dim.leading_zeros()).div_ceil(8) as usize; + buf.push(width as u8); + for &d in chunk_dims.iter().chain(core::iter::once(&element_size)) { + buf.extend_from_slice(&d.to_le_bytes()[..width]); + } +} + fn layout_v4_chunked_prefix(chunk_dims: &[u32], element_size: u32) -> Vec { let mut buf = Vec::new(); buf.push(4); // version @@ -476,35 +463,7 @@ fn layout_v4_chunked_prefix(chunk_dims: &[u32], element_size: u32) -> Vec { let ndims = chunk_dims.len() as u8 + 1; buf.push(ndims); - let max_dim = chunk_dims - .iter() - .map(|&d| d as u64) - .chain(core::iter::once(element_size as u64)) - .max() - .unwrap_or(1); - let dim_encoded_len: u8 = if max_dim <= 0xFF { - 1 - } else if max_dim <= 0xFFFF { - 2 - } else { - 4 - }; - buf.push(dim_encoded_len); - - for &d in chunk_dims { - match dim_encoded_len { - 1 => buf.push(d as u8), - 2 => buf.extend_from_slice(&(d as u16).to_le_bytes()), - 4 => buf.extend_from_slice(&d.to_le_bytes()), - _ => {} - } - } - match dim_encoded_len { - 1 => buf.push(element_size as u8), - 2 => buf.extend_from_slice(&(element_size as u16).to_le_bytes()), - 4 => buf.extend_from_slice(&element_size.to_le_bytes()), - _ => {} - } + push_v4_chunk_dims(&mut buf, chunk_dims, element_size); buf } diff --git a/crates/clawhdf5-format/src/data_layout.rs b/crates/clawhdf5-format/src/data_layout.rs index 60a8243..59a9065 100644 --- a/crates/clawhdf5-format/src/data_layout.rs +++ b/crates/clawhdf5-format/src/data_layout.rs @@ -24,6 +24,34 @@ pub struct VdsMapping { pub virtual_selection: Vec, } +/// Most dimensions a layout message can list (libhdf5 `H5O_LAYOUT_NDIMS`): +/// 32 dataspace dimensions plus the element size. +const MAX_LAYOUT_NDIMS: usize = 33; + +/// libhdf5's checks on a chunked layout message's dimensions +/// (`H5O__layout_decode`): at most [`MAX_LAYOUT_NDIMS`], no dimension 0, and +/// before version 4 at least one dataspace dimension plus the element size. +/// A zero chunk dimension used to read the dataset as all fill values. +fn check_chunk_dims(dims: Vec, layout_version: u8) -> Result, FormatError> { + if dims.len() > MAX_LAYOUT_NDIMS { + return Err(FormatError::InvalidChunkDimensions( + "dimensionality is too large".into(), + )); + } + if layout_version < 4 && dims.len() < 2 { + return Err(FormatError::InvalidChunkDimensions( + "bad dimensions for chunked storage".into(), + )); + } + if let Some(u) = dims.iter().position(|&d| d == 0) { + return Err(FormatError::InvalidChunkDimensions(format!( + "bad chunk dimension value when parsing layout message - chunk dimension must be \ + positive: mesg->u.chunk.dim[{u}] = 0" + ))); + } + Ok(dims) +} + /// Parsed HDF5 data layout message. #[derive(Debug, Clone, PartialEq)] pub enum DataLayout { @@ -394,7 +422,7 @@ impl DataLayout { Ok(DataLayout::Contiguous { address, size }) } _ => Ok(DataLayout::Chunked { - chunk_dimensions: dims, + chunk_dimensions: check_chunk_dims(dims, 2)?, btree_address: address, version: 3, chunk_index_type: None, @@ -457,7 +485,7 @@ impl DataLayout { p += 4; } Ok(DataLayout::Chunked { - chunk_dimensions, + chunk_dimensions: check_chunk_dims(chunk_dimensions, 3)?, btree_address, version: 3, chunk_index_type: None, @@ -506,47 +534,40 @@ impl DataLayout { let dimensionality = data[pos + 1] as usize; let dim_size_encoded_length = data[pos + 2] as usize; let mut p = pos + 3; + if dimensionality > MAX_LAYOUT_NDIMS { + return Err(FormatError::InvalidChunkDimensions( + "dimensionality is too large".into(), + )); + } - // dimension sizes + // Each dimension takes 1 to 8 bytes (libhdf5 writes the + // fewest that hold the largest one, so 3, 5, 6 and 7 occur: + // a chunk dimension of 70 000 takes 3). libhdf5 refuses 0 + // and more than 8. + if dim_size_encoded_length == 0 || dim_size_encoded_length > 8 { + return Err(FormatError::InvalidChunkDimensions( + "encoded chunk dimension size is too large".into(), + )); + } ensure_len(data, p, dimensionality * dim_size_encoded_length)?; let mut chunk_dimensions = Vec::with_capacity(dimensionality); for _ in 0..dimensionality { - let val = match dim_size_encoded_length { - 1 => data[p] as u32, - 2 => u16::from_le_bytes([data[p], data[p + 1]]) as u32, - 4 => u32::from_le_bytes([data[p], data[p + 1], data[p + 2], data[p + 3]]), - 8 => { - // V4 chunked encodes dimension sizes as 8 bytes, but - // our ChunkedStorageV4 stores them as u32. We read only - // the low 4 bytes (little-endian). This silently - // truncates dimensions > 4 GiB, which are not expected - // in practice (HDF5 chunk dimensions are always small). - // If the high bytes are non-zero, the file is malformed - // or uses dimensions we cannot represent. - let high = u32::from_le_bytes([ - data[p + 4], - data[p + 5], - data[p + 6], - data[p + 7], - ]); - if high != 0 { - return Err(FormatError::UnexpectedEof { - expected: p + 8, - available: data.len(), - }); - } - u32::from_le_bytes([data[p], data[p + 1], data[p + 2], data[p + 3]]) - } - _ => { - return Err(FormatError::UnexpectedEof { - expected: p + dim_size_encoded_length, - available: data.len(), - }); - } - }; + let val = data[p..p + dim_size_encoded_length] + .iter() + .rev() + .fold(0u64, |acc, &b| (acc << 8) | u64::from(b)); + // Chunk dimensions are held as u32; HDF5 2.0 can write + // larger ones (layout version 5), which are refused + // rather than truncated. + let val = u32::try_from(val).map_err(|_| { + FormatError::InvalidChunkDimensions(format!( + "chunk dimension {val} is larger than 2^32 - 1, which is not supported" + )) + })?; chunk_dimensions.push(val); p += dim_size_encoded_length; } + let chunk_dimensions = check_chunk_dims(chunk_dimensions, 4)?; // chunk index type ensure_len(data, p, 1)?; @@ -755,6 +776,100 @@ mod tests { ); } + /// A v3 chunked layout message with these dims (element size last). + fn v3_chunked_msg(dims: &[u32]) -> Vec { + let mut buf = vec![3u8, 2, dims.len() as u8]; + buf.extend_from_slice(&0x1000u64.to_le_bytes()); + for d in dims { + buf.extend_from_slice(&d.to_le_bytes()); + } + buf + } + + #[test] + fn chunk_dimensions_are_checked_when_the_layout_is_parsed() { + assert!(DataLayout::parse(&v3_chunked_msg(&[4, 4, 8]), 8, 8).is_ok()); + // A zero chunk dimension used to read as all fill values. + let err = DataLayout::parse(&v3_chunked_msg(&[4, 0, 8]), 8, 8).unwrap_err(); + assert!( + matches!(&err, FormatError::InvalidChunkDimensions(m) if m.contains("dim[1] = 0")), + "{err:?}" + ); + // Only the element-size dimension: libhdf5 "bad dimensions". + assert_eq!( + DataLayout::parse(&v3_chunked_msg(&[8]), 8, 8).unwrap_err(), + FormatError::InvalidChunkDimensions("bad dimensions for chunked storage".into()) + ); + assert_eq!( + DataLayout::parse(&v3_chunked_msg(&[1; 34]), 8, 8).unwrap_err(), + FormatError::InvalidChunkDimensions("dimensionality is too large".into()) + ); + // v1/v2 and v4 messages get the zero check too. + let mut v1 = v1v2_header(1, 2, 2); + v1.extend_from_slice(&0x1000u64.to_le_bytes()); + v1.extend_from_slice(&0u32.to_le_bytes()); + v1.extend_from_slice(&8u32.to_le_bytes()); + assert!(matches!( + DataLayout::parse(&v1, 8, 8), + Err(FormatError::InvalidChunkDimensions(_)) + )); + let mut v4 = vec![4u8, 2, 0, 2, 4]; + v4.extend_from_slice(&0u32.to_le_bytes()); + v4.extend_from_slice(&8u32.to_le_bytes()); + v4.push(3); // fixed array index + v4.push(0); // page bits + v4.extend_from_slice(&0x1000u64.to_le_bytes()); + assert!(matches!( + DataLayout::parse(&v4, 8, 8), + Err(FormatError::InvalidChunkDimensions(_)) + )); + } + + /// A v4 chunked layout (fixed array index) whose `dims` are each + /// encoded in `width` bytes. + fn v4_chunked_msg(width: u8, dims: &[u64]) -> Vec { + let mut m = vec![4u8, 2, 0, dims.len() as u8, width]; + for &d in dims { + m.extend_from_slice(&d.to_le_bytes()[..width.min(8) as usize]); + } + m.push(3); // fixed array index + m.push(0); // page bits + m.extend_from_slice(&0x1000u64.to_le_bytes()); + m + } + + #[test] + fn v4_chunk_dimensions_take_1_to_8_bytes() { + // libhdf5 encodes each dimension in the fewest bytes that hold the + // largest: a chunk dimension of 70 000 takes 3, and 3, 5, 6 and 7 + // were refused ("UnexpectedEof"). + for width in 1..=8u8 { + let dims = [if width >= 3 { 70_000 } else { 200 }, 8]; + let layout = DataLayout::parse(&v4_chunked_msg(width, &dims), 8, 8) + .unwrap_or_else(|e| panic!("width {width}: {e:?}")); + assert!( + matches!(&layout, DataLayout::Chunked { chunk_dimensions, .. } + if chunk_dimensions.iter().map(|&d| u64::from(d)).eq(dims)), + "width {width}: {layout:?}" + ); + } + // libhdf5 refuses 0 and more than 8 bytes. + for width in [0u8, 9] { + assert_eq!( + DataLayout::parse(&v4_chunked_msg(width, &[4, 8]), 8, 8).unwrap_err(), + FormatError::InvalidChunkDimensions( + "encoded chunk dimension size is too large".into() + ) + ); + } + // A dimension past u32 cannot be represented and is refused, not + // truncated. + assert!(matches!( + DataLayout::parse(&v4_chunked_msg(5, &[1 << 32, 8]), 8, 8), + Err(FormatError::InvalidChunkDimensions(m)) if m.contains("2^32") + )); + } + #[test] fn v1v2_rejects_bad_class_dimensionality_and_truncation() { assert_eq!( diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index 76c2b0e..b8f2ffe 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -303,6 +303,7 @@ pub fn read_raw_data_selection( use crate::selection::Selection; crate::partial_read::validate(selection, &dataspace.dimensions)?; + crate::chunked_read::check_chunk_element_size(layout, datatype, offset_size)?; // Read only what the selection's bounding box touches when that is // possible; everything below is the decode-everything-then-pick path, @@ -360,6 +361,7 @@ pub fn read_raw_data_selection( chunk_index_type, .. } => { + crate::chunked_read::chunk_geometry(chunk_dimensions, *version, dataspace, elem_size)?; // For chunked data, only read chunks that intersect the selection let chunk_dims: Vec = chunk_dimensions.iter().map(|&d| d as u64).collect(); let rank = dims.len(); @@ -400,10 +402,10 @@ pub fn read_raw_data_selection( } else { // v3: B-tree v1 if let Some(addr) = btree_address { - crate::chunked_read::collect_chunk_info( + crate::chunked_read::collect_chunk_info_checked( file_data, *addr, - rank + 1, + chunk_dimensions, offset_size, length_size, )? diff --git a/crates/clawhdf5-format/src/datatype.rs b/crates/clawhdf5-format/src/datatype.rs index ba85afa..12e427f 100644 --- a/crates/clawhdf5-format/src/datatype.rs +++ b/crates/clawhdf5-format/src/datatype.rs @@ -208,6 +208,31 @@ fn offset_bytes_for_size(compound_size: u32) -> usize { } /// Read an unsigned integer of 1, 2, 4, or 8 bytes (LE). +/// The size field of the datatype message at `pos`, as stored (a +/// variable-length type's stored size is not modelled in [`Datatype`]). +fn stored_type_size(data: &[u8], pos: usize) -> Result { + ensure_len(data, pos, 8)?; + Ok(LittleEndian::read_u32(&data[pos + 4..pos + 8])) +} + +/// libhdf5 refuses an array type of more than `H5S_MAX_RANK` (32) +/// dimensions. +fn check_array_rank(ndims: usize) -> Result<(), FormatError> { + if ndims > 32 { + return Err(invalid("too many dimensions for array datatype")); + } + Ok(()) +} + +/// A zero-sized array dimension makes a zero-sized type, which libhdf5 +/// cannot open ("unable to retrieve size of datatype"). +fn check_array_dims(dims: &[u32]) -> Result<(), FormatError> { + if dims.contains(&0) { + return Err(invalid("zero-sized dimension specified")); + } + Ok(()) +} + fn read_uint(data: &[u8], offset: usize, nbytes: usize) -> Result { ensure_len(data, offset, nbytes)?; let slice = &data[offset..offset + nbytes]; @@ -232,10 +257,104 @@ fn read_uint(data: &[u8], offset: usize, nbytes: usize) -> Result) -> FormatError { + FormatError::InvalidDatatype(why.into()) +} + +/// libhdf5's bounds checks on an integer type's bit offset and precision +/// (`H5O__dtype_decode_helper`): both must lie inside the type. (Newer +/// libhdf5 checks bit fields the same way; HDF5 2.0, which h5py 3.16 ships, +/// does not, and opens such a type.) +fn check_integer_bits(size: u32, bit_offset: u16, bit_precision: u16) -> Result<(), FormatError> { + let bits = u64::from(size) * 8; + if u64::from(bit_offset) >= bits { + return Err(invalid("integer offset out of bounds")); + } + if bit_precision == 0 { + return Err(invalid("precision is zero")); + } + if u64::from(bit_offset) + u64::from(bit_precision) > bits { + return Err(invalid("integer offset+precision out of bounds")); + } + Ok(()) +} + +/// Whether the closed bit ranges `[a0, a1]` and `[b0, b1]` share a bit. +fn ranges_overlap(a0: u64, a1: u64, b0: u64, b1: u64) -> bool { + a0 <= b1 && b0 <= a1 +} + +/// libhdf5's checks on a floating-point type's fields: exponent and mantissa +/// must lie inside the type, be non-empty, and not overlap each other or the +/// sign bit. (libhdf5 does not check a float's bit offset and precision.) +/// +/// One libhdf5 check is left out on purpose: a sign bit position outside the +/// type ("sign bit position out of bounds"). clawhdf5 up to v2.7.0 wrote 63 +/// there for every float, so every `f32` it wrote (every agent store's +/// embeddings) would stop opening. The position is not used to decode an +/// IEEE float, so reading such a type returns the right values. +fn check_float_fields( + size: u32, + sign: u8, + epos: u8, + esize: u8, + mpos: u8, + msize: u8, +) -> Result<(), FormatError> { + let bits = u64::from(size) * 8; + let (sign, epos, esize, mpos, msize) = ( + u64::from(sign), + u64::from(epos), + u64::from(esize), + u64::from(mpos), + u64::from(msize), + ); + if esize == 0 { + return Err(invalid("exponent size can't be zero")); + } + if epos >= bits { + return Err(invalid("exponent starting position out of bounds")); + } + if epos + esize > bits { + return Err(invalid("exponent range out of bounds")); + } + if msize == 0 { + return Err(invalid("mantissa size can't be zero")); + } + if mpos >= bits { + return Err(invalid("mantissa starting position out of bounds")); + } + if mpos + msize > bits { + return Err(invalid("mantissa range out of bounds")); + } + let (e_end, m_end) = (epos + esize - 1, mpos + msize - 1); + if ranges_overlap(sign, sign, epos, e_end) { + return Err(invalid("exponent and sign positions overlap")); + } + if ranges_overlap(sign, sign, mpos, m_end) { + return Err(invalid("mantissa and sign positions overlap")); + } + if ranges_overlap(epos, e_end, mpos, m_end) { + return Err(invalid("mantissa and exponent positions overlap")); + } + Ok(()) +} + impl Datatype { /// Parse a datatype message from raw bytes. /// /// Returns `(Datatype, bytes_consumed)` for recursive parsing. + /// + /// A type libhdf5 refuses to decode is refused here too, with + /// [`FormatError::InvalidDatatype`] carrying libhdf5's reason: size 0, + /// integer/bit-field/float bit fields outside the type or overlapping, + /// a compound with no members, a member outside its compound, a + /// duplicate or overlapping member, an enum whose size differs from its + /// base type's or with an empty name, an array of more than 32 + /// dimensions or a zero-sized one, an unaligned opaque tag length. + /// Reading such a type used to return data from a corrupt file. Checks + /// newer libhdf5 releases add but HDF5 2.0 (h5py 3.16) lacks are left + /// out, so a file h5py opens still opens here. pub fn parse(data: &[u8]) -> Result<(Datatype, usize), FormatError> { Self::parse_with_depth(data, 0) } @@ -259,6 +378,14 @@ impl Datatype { let size = LittleEndian::read_u32(&data[4..8]); let mut pos = 8; + // libhdf5 refuses size 0 for every class. A fixed-length string is + // exempt: clawhdf5 up to v2.7.0 wrote an empty-string attribute + // with a size-0 string type, and refusing it would fail every + // attribute of such objects, while reading it (an empty string) is + // harmless. + if size == 0 && class_id != 3 { + return Err(invalid("invalid datatype size")); + } match class_id { 0 => { @@ -272,6 +399,7 @@ impl Datatype { let signed = (bf0 >> 3) & 0x01 == 1; let bit_offset = LittleEndian::read_u16(&data[pos..pos + 2]); let bit_precision = LittleEndian::read_u16(&data[pos + 2..pos + 4]); + check_integer_bits(size, bit_offset, bit_precision)?; pos += 4; Ok(( Datatype::FixedPoint { @@ -289,13 +417,23 @@ impl Datatype { ensure_len(data, pos, 12)?; let bo_low = bf0 & 0x01; let bo_high = (bf0 >> 6) & 0x01; + // Bit 6 (with bit 0) is VAX order, defined by version 3; libhdf5 + // ignores bit 6 in older versions, which this read as VAX, + // byte-swapping a little-endian float. + let bo_high = if version >= 3 { bo_high } else { 0 }; let byte_order = match (bo_high, bo_low) { (0, 0) => DatatypeByteOrder::LittleEndian, (0, 1) => DatatypeByteOrder::BigEndian, - (1, 0) => DatatypeByteOrder::Vax, + (1, 0) => { + return Err(invalid("bad byte order for datatype message")); + } (1, 1) => DatatypeByteOrder::Vax, _ => unreachable!(), }; + // Bits 4-5: mantissa normalization; 3 is undefined. + if (bf0 >> 4) & 0x03 == 3 { + return Err(invalid("unknown floating-point normalization")); + } let bit_offset = LittleEndian::read_u16(&data[pos..pos + 2]); let bit_precision = LittleEndian::read_u16(&data[pos + 2..pos + 4]); let exponent_location = data[pos + 4]; @@ -303,6 +441,14 @@ impl Datatype { let mantissa_location = data[pos + 6]; let mantissa_size = data[pos + 7]; let exponent_bias = LittleEndian::read_u32(&data[pos + 8..pos + 12]); + check_float_fields( + size, + bf1, + exponent_location, + exponent_size, + mantissa_location, + mantissa_size, + )?; pos += 12; Ok(( Datatype::FloatingPoint { @@ -371,6 +517,10 @@ impl Datatype { 5 => { // Opaque let tag_len = bf0 as usize; + // libhdf5 writes the NUL-padded length, a multiple of 8. + if !tag_len.is_multiple_of(8) { + return Err(invalid("opaque flag field must be aligned")); + } ensure_len(data, pos, tag_len)?; // The stored tag is NUL-padded to a multiple of 8 bytes; the // tag itself ends at the first NUL (libhdf5 reads it with @@ -384,7 +534,45 @@ impl Datatype { 6 => { // Compound let num_members = (bf0 as u16) | ((bf1 as u16) << 8); - let mut members = Vec::with_capacity(num_members as usize); + if num_members == 0 { + return Err(invalid("invalid number of members: 0")); + } + let mut members: Vec = Vec::with_capacity(num_members as usize); + // Each member's size in the compound as libhdf5 decodes it: + // its stored size, times a v1 member's array dimensions. A + // variable-length member takes 4 + offset size + 4 bytes on + // disk, not the 16 of `Datatype::type_size`. + let mut member_sizes: Vec = Vec::with_capacity(num_members as usize); + // libhdf5 checks each member as it is decoded: it must fit in + // the compound (by its own stored size, before a v1 member's + // array dimensions are applied), and must not repeat a name + // or overlap an earlier member (by its final size). + let check_member = |members: &[CompoundMember], + member_sizes: &[u64], + name: &str, + byte_offset: u64, + stored_size: u32, + final_size: u64| + -> Result<(), FormatError> { + if byte_offset + u64::from(stored_size) > u64::from(size) { + return Err(invalid( + "member type extends outside its parent compound type", + )); + } + if let Some(j) = members.iter().position(|m| m.name == name) { + return Err(invalid(format!( + "duplicated compound field name '{name}', for fields {j} and {}", + members.len() + ))); + } + let end = byte_offset + final_size; + if members.iter().zip(member_sizes).any(|(m, &m_size)| { + byte_offset < m.byte_offset + m_size && m.byte_offset < end + }) { + return Err(invalid("member overlaps with previous member")); + } + Ok(()) + }; if (3..=5).contains(&version) { // v3, v4 and v5 share the compact member encoding (name, @@ -396,9 +584,20 @@ impl Datatype { pos += name_len; let byte_offset = read_uint(data, pos, ob)?; pos += ob; + let stored_size = stored_type_size(data, pos)?; let (member_dt, consumed) = Self::parse_with_depth(&data[pos..], depth + 1)?; pos += consumed; + let final_size = u64::from(stored_size); + check_member( + &members, + &member_sizes, + &name, + byte_offset, + stored_size, + final_size, + )?; + member_sizes.push(final_size); members.push(CompoundMember { name, byte_offset, @@ -438,11 +637,11 @@ impl Datatype { let at = pos + 12 + 4 * j; LittleEndian::read_u32(&data[at..at + 4]) == 0 }); - if ndims > 4 || zero_dim { - return Err(FormatError::InvalidDatatypeVersion { - class: class_id, - version, - }); + if ndims > 4 { + return Err(invalid("invalid number of dimensions for array")); + } + if zero_dim { + return Err(invalid("zero-sized dimension specified")); } array_dims = (0..ndims) .map(|j| { @@ -452,15 +651,28 @@ impl Datatype { .collect(); pos += 28; } + let stored_size = stored_type_size(data, pos)?; let (mut member_dt, consumed) = Self::parse_with_depth(&data[pos..], depth + 1)?; pos += consumed; + let final_size = array_dims.iter().fold(u64::from(stored_size), |a, &d| { + a.saturating_mul(u64::from(d)) + }); if !array_dims.is_empty() { member_dt = Datatype::Array { base_type: Box::new(member_dt), dimensions: array_dims, }; } + check_member( + &members, + &member_sizes, + &name, + byte_offset, + stored_size, + final_size, + )?; + member_sizes.push(final_size); members.push(CompoundMember { name, byte_offset, @@ -499,6 +711,9 @@ impl Datatype { let (base_type, base_consumed) = Self::parse_with_depth(&data[pos..], depth + 1)?; pos += base_consumed; let base_size = base_type.type_size(); + if base_size != size { + return Err(invalid("ENUM datatype size does not match parent")); + } let mut members = Vec::with_capacity(num_members as usize); // Enum layout: base_type, then all names (null-terminated), then all values // v1/v2: names are padded to 8-byte boundaries @@ -506,6 +721,9 @@ impl Datatype { let mut member_names = Vec::with_capacity(num_members as usize); for _ in 0..num_members { let (name, name_len) = read_null_terminated_string(data, pos)?; + if name.is_empty() { + return Err(invalid("0 length enum name")); + } if version < 3 { let padded = (name_len + 7) & !7; pos += padded; @@ -566,6 +784,7 @@ impl Datatype { if version == 2 { ensure_len(data, pos, 4)?; let ndims = data[pos] as usize; + check_array_rank(ndims)?; pos += 4; // ndims(1) + reserved(3) ensure_len(data, pos, ndims * 4 + ndims * 4)?; let mut dimensions = Vec::with_capacity(ndims); @@ -573,6 +792,7 @@ impl Datatype { dimensions.push(LittleEndian::read_u32(&data[pos..pos + 4])); pos += 4; } + check_array_dims(&dimensions)?; // skip permutation indices pos += ndims * 4; let (base_type, consumed) = Self::parse_with_depth(&data[pos..], depth + 1)?; @@ -589,6 +809,7 @@ impl Datatype { // type); HDF5 1.14+/2.0 with `libver=latest` emits v5. ensure_len(data, pos, 1)?; let ndims = data[pos] as usize; + check_array_rank(ndims)?; pos += 1; ensure_len(data, pos, ndims * 4)?; let mut dimensions = Vec::with_capacity(ndims); @@ -596,6 +817,7 @@ impl Datatype { dimensions.push(LittleEndian::read_u32(&data[pos..pos + 4])); pos += 4; } + check_array_dims(&dimensions)?; let (base_type, consumed) = Self::parse_with_depth(&data[pos..], depth + 1)?; pos += consumed; Ok(( @@ -652,6 +874,70 @@ impl Datatype { } } + /// [`Self::parse`] for the datatype message of an object whose header + /// has version `header_version`: a version-1 header, which has no + /// checksum, additionally gets [`Self::check_unused_bits`], as libhdf5 + /// does. Use this wherever the header is at hand. + pub fn parse_in_header( + data: &[u8], + header_version: u8, + ) -> Result<(Datatype, usize), FormatError> { + let parsed = Self::parse(data)?; + if header_version == 1 { + parsed.0.check_unused_bits()?; + } + Ok(parsed) + } + + /// libhdf5's guard against a corrupt numeric type in a header without + /// a checksum (`H5T_is_numeric_with_unusual_unused_bits`, HDF5 1.14.4+): + /// an integer, float or bit field wider than a byte whose precision and + /// offset leave more than half its bits unused is taken for corruption + /// (e.g. a 3-bit integer in 4 bytes, `cve-2024-29162`, or a 32-bit float + /// in 65525 bytes, `cve-2024-32614`), anywhere in the type. libhdf5 + /// skips the check for checksummed (version-2) headers and when the + /// file is opened with `H5Pset_relax_file_integrity_checks`; so does + /// [`Self::parse_in_header`], which has no such option. + pub fn check_unused_bits(&self) -> Result<(), FormatError> { + match self { + Datatype::FixedPoint { + size, + bit_offset, + bit_precision, + .. + } + | Datatype::FloatingPoint { + size, + bit_offset, + bit_precision, + .. + } + | Datatype::BitField { + size, + bit_offset, + bit_precision, + .. + } => { + let bits = u64::from(*size) * 8; + let prec = u64::from(*bit_precision); + if *size > 1 && prec < bits && bits > 2 * (prec + u64::from(*bit_offset)) { + return Err(invalid(format!( + "datatype has unusually large # of unused bits (prec = {prec} bits, \ + size = {size} bytes), possibly corrupted file" + ))); + } + Ok(()) + } + Datatype::Compound { members, .. } => members + .iter() + .try_for_each(|m| m.datatype.check_unused_bits()), + Datatype::Enumeration { base_type, .. } + | Datatype::VariableLength { base_type, .. } + | Datatype::Array { base_type, .. } => base_type.check_unused_bits(), + _ => Ok(()), + } + } + /// Serialize datatype to HDF5 message bytes. pub fn serialize(&self) -> Vec { match self { @@ -863,9 +1149,23 @@ impl Datatype { } /// Check that this datatype can be written: every part of it has an - /// on-disk encoding. [`Self::serialize`] cannot report errors, so the - /// writer calls this first. + /// on-disk encoding, and the encoding is one the reader (and libhdf5) + /// accepts. [`Self::serialize`] cannot report errors, so the writer calls + /// this first. A compound with no fields or a repeated field name, or an + /// enum member with an empty name, is refused here: libhdf5 and h5py + /// refuse such types, and so does [`Self::parse`], so writing one made a + /// file that could not be read back. pub fn check_encodable(&self) -> Result<(), FormatError> { + self.check_encodable_parts()?; + Self::parse(&self.serialize()).map_err(|e| { + FormatError::SerializationError(format!( + "datatype cannot be written: HDF5 readers refuse it ({e})" + )) + })?; + Ok(()) + } + + fn check_encodable_parts(&self) -> Result<(), FormatError> { match self { Datatype::Opaque { tag, .. } if opaque_tag_text(tag).len() > MAX_OPAQUE_TAG_LEN => { Err(FormatError::SerializationError(format!( @@ -878,10 +1178,10 @@ impl Datatype { )), Datatype::Compound { members, .. } => members .iter() - .try_for_each(|m| m.datatype.check_encodable()), + .try_for_each(|m| m.datatype.check_encodable_parts()), Datatype::Enumeration { base_type, .. } | Datatype::VariableLength { base_type, .. } - | Datatype::Array { base_type, .. } => base_type.check_encodable(), + | Datatype::Array { base_type, .. } => base_type.check_encodable_parts(), _ => Ok(()), } } @@ -985,7 +1285,8 @@ mod tests { ) -> Vec { // LE byte order: bo_low=0, bo_high=0 let bf0 = 0x00u8; - let bf1 = 0x00u8; + // Sign bit: the top bit. + let bf1 = (size * 8 - 1) as u8; // mantissa norm = 2 (MSB not stored) in bits 24-31... wait, that's bf2 let bf2 = 0x02u8; // norm = 2 let mut buf = build_dt_header(1, 1, [bf0, bf1, bf2], size); @@ -1012,7 +1313,7 @@ mod tests { let levels = MAX_DATATYPE_DEPTH as usize + 10; let mut data = Vec::new(); for _ in 0..levels { - data.extend_from_slice(&build_dt_header(9, 3, [0, 0, 0], 0)); + data.extend_from_slice(&build_dt_header(9, 3, [0, 0, 0], 16)); } data.extend_from_slice(&build_fixed_point(4, false, false, 0, 32)); @@ -1026,7 +1327,7 @@ mod tests { let levels = MAX_DATATYPE_DEPTH as usize - 1; let mut data = Vec::new(); for _ in 0..levels { - data.extend_from_slice(&build_dt_header(9, 3, [0, 0, 0], 0)); + data.extend_from_slice(&build_dt_header(9, 3, [0, 0, 0], 16)); } data.extend_from_slice(&build_fixed_point(4, false, false, 0, 32)); @@ -1176,8 +1477,8 @@ mod tests { #[test] fn test_opaque() { - // tag_len = 4, tag = "BLOB" - let mut buf = build_dt_header(5, 1, [4, 0, 0], 64); + // tag = "BLOB"; the stored length is the NUL-padded length, 8 + let mut buf = build_dt_header(5, 1, [8, 0, 0], 64); buf.extend_from_slice(b"BLOB"); // Pad to 8 bytes buf.extend_from_slice(&[0, 0, 0, 0]); @@ -2049,4 +2350,273 @@ mod tests { }; assert_eq!(dt.type_size(), 48); } + /// Every check here mirrors one in libhdf5's `H5O__dtype_decode_helper`; + /// the error text is libhdf5's. + fn invalid_reason(data: &[u8]) -> String { + match Datatype::parse(data) { + Err(FormatError::InvalidDatatype(why)) => why, + other => panic!("expected InvalidDatatype, got {other:?}"), + } + } + + #[test] + fn size_zero_is_refused() { + // cve-2017-17508: a variable-length string member of stored size 0. + let mut data = build_dt_header(9, 1, [1, 0, 0], 0); + data.extend_from_slice(&build_fixed_point(1, false, false, 0, 8)); + assert_eq!(invalid_reason(&data), "invalid datatype size"); + // Except a fixed-length string, which clawhdf5 <= v2.7.0 wrote for an + // empty-string attribute. + assert!(Datatype::parse(&build_dt_header(3, 1, [0, 0, 0], 0)).is_ok()); + assert_eq!( + invalid_reason(&build_fixed_point(0, false, false, 0, 0)), + "invalid datatype size" + ); + } + + #[test] + fn integer_bits_must_lie_inside_the_type() { + assert_eq!( + invalid_reason(&build_fixed_point(4, false, false, 32, 1)), + "integer offset out of bounds" + ); + assert_eq!( + invalid_reason(&build_fixed_point(4, false, false, 0, 0)), + "precision is zero" + ); + assert_eq!( + invalid_reason(&build_fixed_point(4, false, false, 8, 25)), + "integer offset+precision out of bounds" + ); + // A partial-precision integer inside its bytes is fine. + assert!(Datatype::parse(&build_fixed_point(4, false, false, 12, 8)).is_ok()); + } + + #[test] + fn float_fields_must_lie_inside_the_type_and_not_overlap() { + // (sign, epos, esize, mpos, msize) on an f32 + let f32_with = |sign: u8, epos: u8, esize: u8, mpos: u8, msize: u8| { + let mut data = build_dt_header(1, 1, [0x20, sign, 0], 4); + data.extend_from_slice(&0u16.to_le_bytes()); + data.extend_from_slice(&32u16.to_le_bytes()); + data.extend_from_slice(&[epos, esize, mpos, msize]); + data.extend_from_slice(&127u32.to_le_bytes()); + data + }; + assert!(Datatype::parse(&f32_with(31, 23, 8, 0, 23)).is_ok()); + for (fields, why) in [ + ((31, 23, 0, 0, 23), "exponent size can't be zero"), + ( + (31, 32, 8, 0, 23), + "exponent starting position out of bounds", + ), + ((31, 30, 8, 0, 23), "exponent range out of bounds"), + ((31, 23, 8, 0, 0), "mantissa size can't be zero"), + ( + (31, 23, 8, 40, 1), + "mantissa starting position out of bounds", + ), + // cve-2024-29163: a 128-bit mantissa in a 4-byte float. + ((31, 23, 8, 0, 128), "mantissa range out of bounds"), + ((23, 23, 8, 0, 23), "exponent and sign positions overlap"), + ((0, 23, 8, 0, 23), "mantissa and sign positions overlap"), + // cve-2026-34734. + ( + (31, 20, 8, 0, 23), + "mantissa and exponent positions overlap", + ), + ] { + let (sign, epos, esize, mpos, msize) = fields; + assert_eq!( + invalid_reason(&f32_with(sign, epos, esize, mpos, msize)), + why, + "{fields:?}" + ); + } + // Normalization 3 is undefined; bit 6 (VAX) needs bit 0 from v3. + let mut data = f32_with(31, 23, 8, 0, 23); + data[1] = 0x30; + assert_eq!( + invalid_reason(&data), + "unknown floating-point normalization" + ); + let mut data = f32_with(31, 23, 8, 0, 23); + data[0] = 0x31; // version 3 + data[1] = 0x60; + assert_eq!(invalid_reason(&data), "bad byte order for datatype message"); + } + + #[test] + fn unusual_unused_bits_are_refused_in_version_1_headers_only() { + // cve-2024-29162: a 3-bit integer in 4 bytes. + let data = build_fixed_point(4, false, true, 0, 3); + assert!(Datatype::parse_in_header(&data, 2).is_ok()); + assert_eq!( + match Datatype::parse_in_header(&data, 1) { + Err(FormatError::InvalidDatatype(why)) => why, + other => panic!("{other:?}"), + }, + "datatype has unusually large # of unused bits (prec = 3 bits, size = 4 bytes), \ + possibly corrupted file" + ); + // Half the bits used (with the offset) is not unusual; nor is a + // 1-byte type; nor a full-precision one. + for (size, offset, prec) in [(4u32, 0u16, 16u16), (4, 8, 8), (1, 0, 1), (8, 0, 64)] { + let data = build_fixed_point(size, false, true, offset, prec); + assert!( + Datatype::parse_in_header(&data, 1).is_ok(), + "{size} {offset} {prec}" + ); + } + // Nested: a compound member's type is checked too. + let member = build_fixed_point(4, false, true, 0, 15); + let data = compound_v3(4, &[("a", 0, member)]); + assert!(Datatype::parse_in_header(&data, 2).is_ok()); + assert!(Datatype::parse_in_header(&data, 1).is_err()); + } + + #[test] + fn f32_written_by_clawhdf5_up_to_2_7_0_still_parses() { + // Those versions put the sign bit at 63 whatever the float's size; + // libhdf5 refuses it ("sign bit position out of bounds"). + let mut data = build_dt_header(1, 1, [0x20, 63, 0], 4); + data.extend_from_slice(&0u16.to_le_bytes()); + data.extend_from_slice(&32u16.to_le_bytes()); + data.extend_from_slice(&[23, 8, 0, 23]); + data.extend_from_slice(&127u32.to_le_bytes()); + assert!(Datatype::parse(&data).is_ok()); + } + + #[test] + fn float_bit_6_is_vax_order_only_from_version_3() { + // h5py opens a v1 float with bit 6 set as an ordinary little-endian + // float; it used to be read as VAX order. + let mut data = build_float(4, 23, 8, 0, 23, 127); + data[1] |= 0x40; + match Datatype::parse(&data).unwrap().0 { + Datatype::FloatingPoint { byte_order, .. } => { + assert_eq!(byte_order, DatatypeByteOrder::LittleEndian) + } + other => panic!("{other:?}"), + } + data[0] = 0x31; + data[1] |= 0x01; + match Datatype::parse(&data).unwrap().0 { + Datatype::FloatingPoint { byte_order, .. } => { + assert_eq!(byte_order, DatatypeByteOrder::Vax) + } + other => panic!("{other:?}"), + } + } + + #[test] + fn opaque_tag_length_must_be_padded() { + let mut data = build_dt_header(5, 1, [4, 0, 0], 4); + data.extend_from_slice(b"BLOB"); + assert_eq!(invalid_reason(&data), "opaque flag field must be aligned"); + } + + /// A v3 compound of `size` bytes with `(name, offset, member)` members. + fn compound_v3(size: u32, members: &[(&str, u8, Vec)]) -> Vec { + let n = members.len() as u8; + let mut data = build_dt_header(6, 3, [n, 0, 0], size); + for (name, off, dt) in members { + data.extend_from_slice(name.as_bytes()); + data.push(0); + data.push(*off); + data.extend_from_slice(dt); + } + data + } + + #[test] + fn compound_members_are_checked() { + let i4 = build_fixed_point(4, false, true, 0, 32); + // cve-2016-4332: no members. + assert_eq!( + invalid_reason(&compound_v3(8, &[])), + "invalid number of members: 0" + ); + assert_eq!( + invalid_reason(&compound_v3( + 8, + &[("a", 0, i4.clone()), ("b", 6, i4.clone())] + )), + "member type extends outside its parent compound type" + ); + assert_eq!( + invalid_reason(&compound_v3( + 8, + &[("a", 0, i4.clone()), ("a", 4, i4.clone())] + )), + "duplicated compound field name 'a', for fields 0 and 1" + ); + assert_eq!( + invalid_reason(&compound_v3( + 8, + &[("a", 0, i4.clone()), ("b", 2, i4.clone())] + )), + "member overlaps with previous member" + ); + assert_eq!( + invalid_reason(&compound_v3( + 8, + &[("b", 4, i4.clone()), ("a", 2, i4.clone())] + )), + "member overlaps with previous member" + ); + // Members out of offset order, and gaps, are fine. + assert!(Datatype::parse(&compound_v3(12, &[("b", 8, i4.clone()), ("a", 0, i4)])).is_ok()); + } + + #[test] + fn enum_is_checked() { + let base = build_fixed_point(4, false, true, 0, 32); + let enum_of = |size: u32, names: &[&str]| { + let mut data = build_dt_header(8, 3, [names.len() as u8, 0, 0], size); + data.extend_from_slice(&base); + for n in names { + data.extend_from_slice(n.as_bytes()); + data.push(0); + } + for i in 0..names.len() as u32 { + data.extend_from_slice(&i.to_le_bytes()); + } + data + }; + assert!(Datatype::parse(&enum_of(4, &["RED", "GREEN"])).is_ok()); + // cve-2024-32618. + assert_eq!( + invalid_reason(&enum_of(4, &["", "GREEN"])), + "0 length enum name" + ); + assert_eq!( + invalid_reason(&enum_of(2, &["RED"])), + "ENUM datatype size does not match parent" + ); + } + + #[test] + fn array_dimensions_are_checked() { + let base = build_fixed_point(4, false, true, 0, 32); + let array_v3 = |dims: &[u32]| { + let n = dims.iter().product::().max(1); + let mut data = build_dt_header(10, 3, [0, 0, 0], 4 * n); + data.push(dims.len() as u8); + for d in dims { + data.extend_from_slice(&d.to_le_bytes()); + } + data.extend_from_slice(&base); + data + }; + assert!(Datatype::parse(&array_v3(&[2, 3])).is_ok()); + assert_eq!( + invalid_reason(&array_v3(&[2, 0])), + "zero-sized dimension specified" + ); + assert_eq!( + invalid_reason(&array_v3(&[1; 33])), + "too many dimensions for array datatype" + ); + } } diff --git a/crates/clawhdf5-format/src/ea_writer.rs b/crates/clawhdf5-format/src/ea_writer.rs index e0f0375..f669534 100644 --- a/crates/clawhdf5-format/src/ea_writer.rs +++ b/crates/clawhdf5-format/src/ea_writer.rs @@ -7,7 +7,9 @@ extern crate alloc; use alloc::{vec, vec::Vec}; use crate::checksum::jenkins_lookup3; -use crate::chunked_write::{WrittenChunk, filtered_chunk_size_len, push_addr, push_index_element}; +use crate::chunked_write::{ + WrittenChunk, filtered_chunk_size_len, push_addr, push_index_element, push_v4_chunk_dims, +}; /// Serialize a v4 Extensible Array layout message. pub(crate) fn serialize_v4_extensible_array( @@ -24,35 +26,7 @@ pub(crate) fn serialize_v4_extensible_array( let ndims = chunk_dims.len() as u8 + 1; buf.push(ndims); - let max_dim = chunk_dims - .iter() - .map(|&d| d as u64) - .chain(core::iter::once(element_size as u64)) - .max() - .unwrap_or(1); - let dim_encoded_len: u8 = if max_dim <= 0xFF { - 1 - } else if max_dim <= 0xFFFF { - 2 - } else { - 4 - }; - buf.push(dim_encoded_len); - - for &d in chunk_dims { - match dim_encoded_len { - 1 => buf.push(d as u8), - 2 => buf.extend_from_slice(&(d as u16).to_le_bytes()), - 4 => buf.extend_from_slice(&d.to_le_bytes()), - _ => unreachable!("unexpected dim_encoded_len: {dim_encoded_len}"), - } - } - match dim_encoded_len { - 1 => buf.push(element_size as u8), - 2 => buf.extend_from_slice(&(element_size as u16).to_le_bytes()), - 4 => buf.extend_from_slice(&element_size.to_le_bytes()), - _ => unreachable!("unexpected dim_encoded_len: {dim_encoded_len}"), - } + push_v4_chunk_dims(&mut buf, chunk_dims, element_size); // chunk index type = 4 (Extensible Array) buf.push(4); diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index bb81939..a9a0c2b 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -201,6 +201,28 @@ pub enum FormatError { DuplicateDatasetName(String), /// Integer overflow in size computation (malformed data protection). Overflow(String), + /// An object header that libhdf5 refuses to load (the reason is + /// libhdf5's own error text): a misaligned or overrunning message, a + /// wrong message count, contradictory message flags, a message of a + /// class that cannot be shared flagged shareable, … + InvalidObjectHeader(&'static str), + /// A datatype message libhdf5 refuses to decode (the reason is + /// libhdf5's own error text): size 0, bit fields outside the type, + /// an empty enum name, a compound member outside its compound, … + InvalidDatatype(String), + /// A chunked layout whose chunk dimensions libhdf5 refuses: a zero + /// dimension, a rank that does not match the dataspace, an element size + /// that is not the datatype's, or a chunk of 4 GiB or more indexed by a + /// version-1 B-tree. + InvalidChunkDimensions(String), + /// The superblock's end-of-file address lies past the end of the file: + /// the file was truncated (libhdf5 refuses to open it). + TruncatedFile { + /// End of file recorded in the superblock (relative to byte 0). + stored_eof: u64, + /// The file's actual length in bytes. + actual_len: u64, + }, } impl fmt::Display for FormatError { @@ -445,6 +467,25 @@ impl fmt::Display for FormatError { FormatError::Overflow(msg) => { write!(f, "integer overflow: {msg}") } + FormatError::InvalidObjectHeader(why) => { + write!(f, "corrupt object header: {why}") + } + FormatError::InvalidDatatype(why) => { + write!(f, "invalid datatype: {why}") + } + FormatError::InvalidChunkDimensions(why) => { + write!(f, "invalid chunk dimensions: {why}") + } + FormatError::TruncatedFile { + stored_eof, + actual_len, + } => { + write!( + f, + "truncated file: the superblock records end of file {stored_eof}, \ + but the file is {actual_len} bytes" + ) + } } } } diff --git a/crates/clawhdf5-format/src/object_header.rs b/crates/clawhdf5-format/src/object_header.rs index 4b8067f..2c3c819 100644 --- a/crates/clawhdf5-format/src/object_header.rs +++ b/crates/clawhdf5-format/src/object_header.rs @@ -108,10 +108,20 @@ impl ObjectHeader { return Err(FormatError::InvalidObjectHeaderVersion(version)); } - let num_messages = LittleEndian::read_u16(&data[offset + 2..offset + 4]); + let num_messages = LittleEndian::read_u16(&data[offset + 2..offset + 4]) as usize; let reference_count = LittleEndian::read_u32(&data[offset + 4..offset + 8]); let header_data_size = LittleEndian::read_u32(&data[offset + 8..offset + 12]) as usize; + // libhdf5 (H5O__prefix_deserialize): a header with messages needs room + // for at least one message header, and one without has an empty chunk. + if (num_messages > 0 && header_data_size < V1_MSG_HEADER_SIZE) + || (num_messages == 0 && header_data_size > 0) + { + return Err(FormatError::InvalidObjectHeader( + "bad object header chunk size", + )); + } + // Pad to 8-byte alignment: header prefix is 12 bytes, pad to 16 let padding = 4; // pad 12-byte prefix to 16-byte alignment let msg_start = offset @@ -124,64 +134,23 @@ impl ObjectHeader { ensure_len(data, msg_start, header_data_size)?; let mut messages = Vec::new(); - let mut pos = msg_start; - let msg_end = - msg_start - .checked_add(header_data_size) - .ok_or(FormatError::UnexpectedEof { - expected: usize::MAX, - available: data.len(), - })?; - - for _ in 0..num_messages { - if pos + 8 > msg_end { - break; - } - 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 += 8; - - ensure_len(data, pos, msg_data_size)?; - let msg_type = MessageType::from_u16(msg_type_raw); - - check_unknown_message(msg_type, msg_flags)?; - - if msg_type != MessageType::Nil { - messages.push(HeaderMessage { - msg_type, - size: msg_data_size, - flags: msg_flags, - creation_order: None, - data: data[pos..pos + msg_data_size].to_vec(), - }); - } - - pos += msg_data_size; - - // Follow continuations - if msg_type == MessageType::ObjectHeaderContinuation { - let cont_msg_data = &messages - .last() - .ok_or(FormatError::InvalidObjectHeaderSignature)? - .data; - if cont_msg_data.len() >= (offset_size as usize + length_size as usize) { - let cont_offset = read_offset(cont_msg_data, 0, offset_size)? as usize; - let cont_length = - read_offset(cont_msg_data, offset_size as usize, length_size)? as usize; - // Parse continuation block (v1: just raw messages, no signature) - let cont_msgs = Self::parse_v1_continuation( - data, - cont_offset, - cont_length, - offset_size, - length_size, - 32, // max continuation depth - )?; - messages.extend(cont_msgs); - } - } + let chunk0_count = Self::parse_v1_chunk( + data, + msg_start, + header_data_size, + offset_size, + length_size, + MAX_V1_CONTINUATION_DEPTH, + &mut messages, + )?; + // libhdf5 reads every message in the first chunk and refuses a header + // whose prefix claims fewer than that (continuation chunks are read + // later and not held to the count). Stopping after the claimed number + // silently dropped the rest. + if chunk0_count > num_messages { + return Err(FormatError::InvalidObjectHeader( + "bad object header message count", + )); } Ok(ObjectHeader { @@ -196,72 +165,87 @@ impl ObjectHeader { }) } - fn parse_v1_continuation( + /// Parse the messages of one version-1 chunk (`length` bytes at + /// `offset`, no signature), following continuation messages as they are + /// met. Returns how many messages (NIL ones included) this chunk itself + /// holds. + /// + /// A version-1 chunk is filled with messages whose sizes are multiples of + /// 8; libhdf5 refuses a message that is not aligned, that runs past the + /// end of the chunk, or leftover bytes too few for a message header (a + /// "gap", which only version 2 allows). + #[allow(clippy::too_many_arguments)] + fn parse_v1_chunk( data: &[u8], offset: usize, length: usize, offset_size: u8, length_size: u8, depth_remaining: u16, - ) -> Result, FormatError> { + messages: &mut Vec, + ) -> Result { if depth_remaining == 0 { return Err(FormatError::NestingDepthExceeded); } ensure_len(data, offset, length)?; - let mut messages = Vec::new(); + let end = offset + length; let mut pos = offset; - let end = offset.saturating_add(length); + let mut count = 0usize; - while pos + 8 <= end { + while pos < end { + if end - pos < V1_MSG_HEADER_SIZE { + return Err(FormatError::InvalidObjectHeader( + "gap found in early version of file format", + )); + } 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]; - pos += 8; + // reserved(3) at pos+5..pos+8 + pos += V1_MSG_HEADER_SIZE; - if pos + msg_data_size > end { - break; + 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); - - check_unknown_message(msg_type, msg_flags)?; - if msg_type != MessageType::Nil { messages.push(HeaderMessage { msg_type, size: msg_data_size, flags: msg_flags, creation_order: None, - data: data[pos..pos + msg_data_size].to_vec(), + data: body.to_vec(), }); } - pos += msg_data_size; - // Recursive continuations + // Follow continuations (v1 continuation chunks are just raw + // messages, no signature); check_message has checked the body. if msg_type == MessageType::ObjectHeaderContinuation { - let cont_msg_data = &messages - .last() - .ok_or(FormatError::InvalidObjectHeaderSignature)? - .data; - if cont_msg_data.len() >= (offset_size as usize + length_size as usize) { - let cont_offset = read_offset(cont_msg_data, 0, offset_size)? as usize; - let cont_length = - read_offset(cont_msg_data, offset_size as usize, length_size)? as usize; - let cont_msgs = Self::parse_v1_continuation( - data, - cont_offset, - cont_length, - offset_size, - length_size, - depth_remaining - 1, - )?; - messages.extend(cont_msgs); - } + let cont_offset = read_offset(body, 0, offset_size)? as usize; + let cont_length = read_offset(body, offset_size as usize, length_size)? as usize; + Self::parse_v1_chunk( + data, + cont_offset, + cont_length, + offset_size, + length_size, + depth_remaining - 1, + messages, + )?; } } - Ok(messages) + Ok(count) } fn parse_v2( @@ -278,6 +262,11 @@ impl ObjectHeader { return Err(FormatError::InvalidObjectHeaderVersion(version)); } let flags = data[offset + 5]; + if flags & !V2_HDR_ALL_FLAGS != 0 { + return Err(FormatError::InvalidObjectHeader( + "unknown object header status flag(s)", + )); + } let mut pos = offset + 6; @@ -297,7 +286,14 @@ impl ObjectHeader { // Optional attribute storage thresholds (flags bit 4) if flags & 0x10 != 0 { ensure_len(data, pos, 4)?; - // max_compact_attrs(2) + min_dense_attrs(2) — read but don't store for now + // max_compact_attrs(2) + min_dense_attrs(2) — checked, not stored + let max_compact = LittleEndian::read_u16(&data[pos..pos + 2]); + let min_dense = LittleEndian::read_u16(&data[pos + 2..pos + 4]); + if max_compact < min_dense { + return Err(FormatError::InvalidObjectHeader( + "bad object header attribute phase change values", + )); + } pos += 4; } @@ -312,6 +308,14 @@ impl ObjectHeader { ensure_len(data, pos, chunk_size_width as usize)?; let chunk0_size = read_offset(data, pos, chunk_size_width)? as usize; pos += chunk_size_width as usize; + // Bit 2: attribute creation order tracked → messages include creation order field + let has_creation_order = flags & 0x04 != 0; + let msg_header_size = if has_creation_order { 6 } else { 4 }; + if chunk0_size > 0 && chunk0_size < msg_header_size { + return Err(FormatError::InvalidObjectHeader( + "bad object header chunk size", + )); + } let chunk0_msg_start = pos; let chunk0_msg_end = pos @@ -335,9 +339,6 @@ impl ObjectHeader { } } - // Bit 2: attribute creation order tracked → messages include creation order field - let has_creation_order = flags & 0x04 != 0; - // Parse messages from chunk0 let mut messages = Vec::new(); let mut continuations = Vec::new(); @@ -396,8 +397,20 @@ impl ObjectHeader { ) -> Result<(), FormatError> { let msg_header_size = if has_creation_order { 6 } else { 4 }; let mut pos = start; + let mut null_count = 0usize; - while pos + msg_header_size <= end { + while pos < end { + // Leftover bytes too few for a message header are a gap, which + // libhdf5 allows only in a chunk without NIL messages (a writer + // that leaves a gap had no NIL message to put the space in). + if end - pos < msg_header_size { + if null_count != 0 { + return Err(FormatError::InvalidObjectHeader( + "gap in chunk with no null messages", + )); + } + break; + } let msg_type_raw = data[pos] as u16; let msg_data_size = LittleEndian::read_u16(&data[pos + 1..pos + 3]) as usize; let msg_flags = data[pos + 3]; @@ -408,32 +421,36 @@ impl ObjectHeader { }; pos += msg_header_size; - if pos + msg_data_size > end { - // Could be padding at end of chunk - break; + // `end` is where the messages stop and the checksum starts. + // libhdf5 bounds a message by the chunk including its checksum, + // but a message that runs into the checksum still fails there: + // its loop stops at the checksum, and reading the checksum from + // past its start overruns the chunk ("ran off end of input + // buffer while decoding"). Both refuse it; only the text + // differs. + 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(2, msg_type_raw, msg_flags, body, offset_size, length_size)?; let msg_type = MessageType::from_u16(msg_type_raw); - - check_unknown_message(msg_type, msg_flags)?; - - let msg_data = data[pos..pos + msg_data_size].to_vec(); - if msg_type == MessageType::ObjectHeaderContinuation { - // Parse continuation offset/length from message data - if msg_data.len() >= (offset_size as usize + length_size as usize) { - let cont_off = read_offset(&msg_data, 0, offset_size)? as usize; - let cont_len = - read_offset(&msg_data, offset_size as usize, length_size)? as usize; - continuations.push((cont_off, cont_len)); - } - } else if msg_type != MessageType::Nil { + // check_message has checked the body holds both fields. + let cont_off = read_offset(body, 0, offset_size)? as usize; + let cont_len = read_offset(body, offset_size as usize, length_size)? as usize; + continuations.push((cont_off, cont_len)); + } else if msg_type == MessageType::Nil { + null_count += 1; + } else { messages.push(HeaderMessage { msg_type, size: msg_data_size, flags: msg_flags, creation_order, - data: msg_data, + data: body.to_vec(), }); } @@ -496,22 +513,149 @@ impl ObjectHeader { } } -/// Header message flag bit 7: fail if the message is unknown, always. +/// Size of a version-1 message header: type(2) + size(2) + flags(1) + reserved(3). +const V1_MSG_HEADER_SIZE: usize = 8; + +/// How deep version-1 continuation chunks may chain (malformed-data guard). +const MAX_V1_CONTINUATION_DEPTH: u16 = 32; + +/// Every defined version-2 object header status flag (libhdf5 +/// `H5O_HDR_ALL_FLAGS`): chunk-0 size width (bits 0-1), attribute creation +/// order tracked/indexed, attribute phase-change values, times stored. +const V2_HDR_ALL_FLAGS: u8 = 0x3F; + +// Header message flag bits (libhdf5 `H5O_MSG_FLAG_*`). Bit 0 (constant) needs +// no check. Bit 3 (fail if unknown and the file is opened for writing) never +// fails a read: the parser only ever reads, as libhdf5 ignores it for a +// read-only open. +const MSG_FLAG_SHARED: u8 = 0x02; +const MSG_FLAG_DONTSHARE: u8 = 0x04; +const MSG_FLAG_FAIL_IF_UNKNOWN_AND_OPEN_FOR_WRITE: u8 = 0x08; +const MSG_FLAG_MARK_IF_UNKNOWN: u8 = 0x10; +const MSG_FLAG_WAS_UNKNOWN: u8 = 0x20; +const MSG_FLAG_SHAREABLE: u8 = 0x40; +/// Fail if the message is unknown, whatever the access mode. const MSG_FLAG_FAIL_IF_UNKNOWN_ALWAYS: u8 = 0x80; -/// Refuse an unknown message the file says no reader may skip. +/// Message type ids libhdf5 has a class for (`H5O_msg_class_g`): 0x00-0x18 +/// except 0x09 (a test-only "bogus" message). Anything else is an unknown +/// message. +fn is_known_message(id: u16) -> bool { + id <= 0x18 && id != 0x09 +} + +/// Message classes that may be shared (`H5O_SHARE_IS_SHARABLE`): dataspace, +/// datatype, the two fill-value messages, filter pipeline and attribute. +fn is_shareable_message(id: u16) -> bool { + matches!(id, 0x01 | 0x03 | 0x04 | 0x05 | 0x0B | 0x0C) +} + +/// Check one header message the way libhdf5 does while it loads an object +/// header (`H5O__chunk_deserialize`), so an object libhdf5 refuses to open is +/// refused here too instead of being read from a corrupt header: /// -/// The parser only ever reads, so bit 3 (fail only when opened for writing) -/// is ignored, as libhdf5 ignores it for a read-only open; bit 7 fails -/// regardless of access mode. This had the two the wrong way round, failing -/// objects libhdf5 reads and reading ones it refuses (`tbogus.h5`). -fn check_unknown_message(msg_type: MessageType, msg_flags: u8) -> Result<(), FormatError> { - match msg_type { - MessageType::Unknown(id) if msg_flags & MSG_FLAG_FAIL_IF_UNKNOWN_ALWAYS != 0 => { - Err(FormatError::UnsupportedMessage(id)) - } - _ => Ok(()), +/// - contradictory flag combinations; +/// - an unknown message the file says no reader may skip (bit 7). This had +/// bits 3 and 7 the wrong way round once, failing objects libhdf5 reads +/// and reading ones it refuses (`tbogus.h5`); +/// - a known message whose class cannot be shared, flagged shared or +/// shareable (`cve-2016-4332`); +/// - the messages libhdf5 decodes while loading the header, whose decode +/// errors fail the load: continuation, reference count (which a version-1 +/// header cannot hold), and both modification-time messages. +fn check_message( + header_version: u8, + id: u16, + flags: u8, + body: &[u8], + offset_size: u8, + length_size: u8, +) -> Result<(), FormatError> { + let bad_flags = FormatError::InvalidObjectHeader("bad flag combination for message"); + if flags & MSG_FLAG_SHARED != 0 && flags & MSG_FLAG_DONTSHARE != 0 { + return Err(bad_flags); } + if flags & MSG_FLAG_WAS_UNKNOWN != 0 + && (flags & MSG_FLAG_FAIL_IF_UNKNOWN_AND_OPEN_FOR_WRITE != 0 + || flags & MSG_FLAG_MARK_IF_UNKNOWN == 0) + { + return Err(bad_flags); + } + + if !is_known_message(id) { + if flags & MSG_FLAG_FAIL_IF_UNKNOWN_ALWAYS != 0 { + return Err(FormatError::UnsupportedMessage(id)); + } + return Ok(()); + } + if flags & (MSG_FLAG_SHARED | MSG_FLAG_SHAREABLE) != 0 && !is_shareable_message(id) { + return Err(FormatError::InvalidObjectHeader( + "message of unshareable class flagged as shareable", + )); + } + + let overrun = FormatError::InvalidObjectHeader("ran off end of input buffer while decoding"); + match id { + // Continuation: address + length, and the chunk cannot be empty. + 0x10 => { + if body.len() < offset_size as usize + length_size as usize { + return Err(overrun); + } + if read_offset(body, offset_size as usize, length_size)? == 0 { + return Err(FormatError::InvalidObjectHeader( + "invalid continuation chunk size (0)", + )); + } + } + // Reference count: version-2 headers only; version 0 then a u32. + 0x16 => { + if header_version == 1 { + return Err(FormatError::InvalidObjectHeader( + "object header version does not support reference count message", + )); + } + match body.first() { + None => return Err(overrun), + Some(0) => {} + Some(_) => { + return Err(FormatError::InvalidObjectHeader( + "bad version number for reference count message", + )); + } + } + if body.len() < 5 { + return Err(overrun); + } + } + // Old modification time: "YYYYMMDDhhmmss" and 2 reserved bytes. + 0x0E => { + if body.len() < 16 { + return Err(overrun); + } + if !body[..14].iter().all(u8::is_ascii_digit) { + return Err(FormatError::InvalidObjectHeader( + "badly formatted modification time message", + )); + } + } + // New modification time: version 1, 3 reserved bytes, u32 seconds. + 0x12 => { + match body.first() { + None => return Err(overrun), + Some(1) => {} + Some(_) => { + return Err(FormatError::InvalidObjectHeader( + "bad version number for mtime message", + )); + } + } + if body.len() < 8 { + return Err(overrun); + } + } + _ => {} + } + Ok(()) } #[cfg(test)] @@ -528,11 +672,14 @@ mod tests { // Calculate total header message data size let mut msg_bytes = Vec::new(); for (mtype, mdata, mflags) in messages { + // v1 message sizes are multiples of 8 (the data is zero-padded). + let padded = mdata.len().div_ceil(8) * 8; msg_bytes.extend_from_slice(&mtype.to_le_bytes()); // type(2) - msg_bytes.extend_from_slice(&(mdata.len() as u16).to_le_bytes()); // size(2) + msg_bytes.extend_from_slice(&(padded as u16).to_le_bytes()); // size(2) msg_bytes.push(*mflags); // flags(1) msg_bytes.extend_from_slice(&[0u8; 3]); // reserved(3) msg_bytes.extend_from_slice(mdata); // data + msg_bytes.resize(msg_bytes.len() + padded - mdata.len(), 0); } let mut buf = Vec::new(); @@ -622,9 +769,10 @@ mod tests { let hdr = ObjectHeader::parse(&data, 0, 8, 8).unwrap(); assert_eq!(hdr.messages.len(), 2); assert_eq!(hdr.messages[0].msg_type, MessageType::Dataspace); - assert_eq!(hdr.messages[0].data, vec![1, 2, 3, 4]); + // v1 message data is padded to a multiple of 8 bytes. + assert_eq!(hdr.messages[0].data, vec![1, 2, 3, 4, 0, 0, 0, 0]); assert_eq!(hdr.messages[1].msg_type, MessageType::DataLayout); - assert_eq!(hdr.messages[1].data, vec![5, 6]); + assert_eq!(hdr.messages[1].data[..2], [5, 6]); } #[test] @@ -650,7 +798,7 @@ mod tests { // Bit 3 = fail if unknown *and the file is opened for writing*. This // parser only reads, so libhdf5 (read-only) opens such an object and // so must we. Bits 4/5 (mark if unknown / was unknown) never fail. - for flags in [0x08u8, 0x10, 0x20, 0x38] { + for flags in [0x08u8, 0x10, 0x30] { let messages = [(0x00FFu16, &[0xAA][..], flags)]; let data = build_v1_header(&messages, 8, 8); let hdr = ObjectHeader::parse(&data, 0, 8, 8).unwrap(); @@ -658,6 +806,231 @@ mod tests { } } + #[test] + fn contradictory_message_flags_are_refused() { + // libhdf5: "bad flag combination for message" for shared + don't + // share, was-unknown without mark-if-unknown, and was-unknown with + // fail-if-unknown-on-write. + for flags in [0x06u8, 0x20, 0x38] { + let data = build_v1_header(&[(0x00FFu16, &[0xAA][..], flags)], 8, 8); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("bad flag combination for message"), + "flags {flags:#x}" + ); + let data = build_v2_header(0x00, &[(0xF0, &[1, 2], flags)], None); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("bad flag combination for message"), + "flags {flags:#x}" + ); + } + } + + #[test] + fn unshareable_message_flagged_shareable_is_refused() { + // A layout (0x08) or modification time (0x12) message cannot be + // shared; bit 1 (shared) or bit 6 (shareable) on one is corruption + // (cve-2016-4332). A datatype (0x03) may be shareable. + let mtime = [1u8, 0, 0, 0, 0x10, 0x20, 0x30, 0x40]; + for (id, flags) in [(0x08u16, 0x40u8), (0x08, 0x02), (0x12, 0x40)] { + let data = build_v1_header(&[(id, &mtime[..], flags)], 8, 8); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader( + "message of unshareable class flagged as shareable" + ), + "id {id:#x} flags {flags:#x}" + ); + } + let data = build_v1_header(&[(0x03, &[0u8; 8][..], 0x40)], 8, 8); + assert!(ObjectHeader::parse(&data, 0, 8, 8).is_ok()); + // An unknown message is never checked for shareability. + let data = build_v1_header(&[(0x00FF, &[0u8; 8][..], 0x40)], 8, 8); + assert!(ObjectHeader::parse(&data, 0, 8, 8).is_ok()); + } + + #[test] + fn v1_message_must_be_aligned() { + // cve-2018-13873: a v1 message whose size is not a multiple of 8. + let mut data = build_v1_header(&[(0x01, &[0u8; 8][..], 0)], 8, 8); + data[16 + 2] = 7; // size field of the only message + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("message not aligned") + ); + } + + #[test] + fn message_overrunning_its_chunk_is_refused() { + // It used to end the chunk quietly, dropping this message and any + // after it. + let mut data = build_v1_header(&[(0x01, &[0u8; 8][..], 0)], 8, 8); + data[16 + 2] = 16; + data.resize(data.len() + 64, 0); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("message size exceeds buffer end") + ); + let mut data = build_v2_header(0x00, &[(0x01, &[1, 2], 0)], None); + data[7 + 1] = 9; // size of the only message (after OHDR, ver, flags, chunk size) + let chk = crate::checksum::jenkins_lookup3(&data[..data.len() - 4]); + let n = data.len(); + data[n - 4..].copy_from_slice(&chk.to_le_bytes()); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("message size exceeds buffer end") + ); + } + + #[test] + fn v1_gap_after_last_message_is_refused() { + // Fewer than 8 bytes left over: a gap, which only version 2 allows. + let mut data = build_v1_header(&[(0x01, &[0u8; 8][..], 0)], 8, 8); + data[8] += 4; // header_data_size + data.extend_from_slice(&[0u8; 4]); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("gap found in early version of file format") + ); + } + + #[test] + fn v1_chunk_holding_more_messages_than_the_prefix_says_is_refused() { + // cve-2024-32619: the prefix says 1 message, the chunk holds 2. The + // second used to be dropped silently. + let mut data = build_v1_header(&[(0x01, &[0u8; 8][..], 0), (0x03, &[0u8; 8][..], 0)], 8, 8); + data[2] = 1; + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("bad object header message count") + ); + // Fewer in the chunk than the prefix says is fine (the rest may be in + // continuation chunks; libhdf5 only enforces that with strict checks). + data[2] = 3; + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap().messages.len(), + 2 + ); + } + + #[test] + fn v1_prefix_chunk_size_must_fit_the_message_count() { + let mut data = build_v1_header(&[], 8, 8); + data[8] = 8; // no messages but a non-empty chunk + data.extend_from_slice(&[0u8; 8]); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("bad object header chunk size") + ); + } + + #[test] + fn v1_header_cannot_hold_a_reference_count_message() { + // cve-2018-11204. + let data = build_v1_header(&[(0x16, &[0, 2, 0, 0, 0][..], 0)], 8, 8); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader( + "object header version does not support reference count message" + ) + ); + let data = build_v2_header(0x00, &[(0x16, &[0, 2, 0, 0, 0], 0)], None); + assert!(ObjectHeader::parse(&data, 0, 8, 8).is_ok()); + } + + #[test] + fn modification_time_messages_are_decoded_with_the_header() { + // cve-2024-33873 (version 0) and cve-2024-33874 (empty message). + for (body, why) in [ + ( + &[0u8, 0, 0, 0, 1, 2, 3, 4][..], + "bad version number for mtime message", + ), + (&[][..], "ran off end of input buffer while decoding"), + ] { + let data = build_v2_header(0x00, &[(0x12, body, 0)], None); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader(why) + ); + } + let data = build_v2_header(0x00, &[(0x12, &[1, 0, 0, 0, 1, 2, 3, 4], 0)], None); + assert!(ObjectHeader::parse(&data, 0, 8, 8).is_ok()); + // The old (0x0E) message is 14 ASCII digits and 2 reserved bytes. + let data = build_v1_header(&[(0x0E, &b"20110414214255\0\0"[..], 0)], 8, 8); + assert!(ObjectHeader::parse(&data, 0, 8, 8).is_ok()); + let data = build_v1_header(&[(0x0E, &b"2011041421425x\0\0"[..], 0)], 8, 8); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("badly formatted modification time message") + ); + } + + #[test] + fn continuation_message_must_hold_a_nonempty_chunk() { + let mut cont = [0u8; 16]; + cont[..8].copy_from_slice(&64u64.to_le_bytes()); + let data = build_v2_header(0x00, &[(0x10, &cont, 0)], None); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("invalid continuation chunk size (0)") + ); + let data = build_v2_header(0x00, &[(0x10, &cont[..8], 0)], None); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("ran off end of input buffer while decoding") + ); + } + + #[test] + fn v2_prefix_is_checked() { + let data = build_v2_header(0x40, &[(0x01, &[1], 0)], None); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("unknown object header status flag(s)") + ); + // build_v2_header writes max_compact 8, min_dense 6; swap them. + let mut data = build_v2_header(0x10, &[(0x01, &[1], 0)], None); + data[6] = 6; + data[8] = 8; + let chk = crate::checksum::jenkins_lookup3(&data[..data.len() - 4]); + let n = data.len(); + data[n - 4..].copy_from_slice(&chk.to_le_bytes()); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("bad object header attribute phase change values") + ); + } + + #[test] + fn v2_gap_is_allowed_only_without_nil_messages() { + // Three bytes after the last message: a gap (a message header is 4). + let mut data = build_v2_header(0x00, &[(0x01, &[1, 2, 3], 0), (0x03, &[], 0)], None); + // Turn the empty datatype message (4 header bytes) into a 3-byte gap + // by shrinking the chunk. + let n = data.len(); + data.truncate(n - 5); + data[6] -= 1; + let chk = crate::checksum::jenkins_lookup3(&data); + data.extend_from_slice(&chk.to_le_bytes()); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap().messages.len(), + 1 + ); + + let mut data = build_v2_header(0x00, &[(0x00, &[1, 2, 3], 0), (0x03, &[], 0)], None); + let n = data.len(); + data.truncate(n - 5); + data[6] -= 1; + let chk = crate::checksum::jenkins_lookup3(&data); + data.extend_from_slice(&chk.to_le_bytes()); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::InvalidObjectHeader("gap in chunk with no null messages") + ); + } + #[test] fn parse_v2_unknown_message_flags() { let data = build_v2_header(0x00, &[(0xF0, &[1, 2], 0x08)], None); diff --git a/crates/clawhdf5-format/src/superblock.rs b/crates/clawhdf5-format/src/superblock.rs index d2ec4d6..a565321 100644 --- a/crates/clawhdf5-format/src/superblock.rs +++ b/crates/clawhdf5-format/src/superblock.rs @@ -100,6 +100,42 @@ pub mod swmr_flags { } impl Superblock { + /// Where the HDF5 data ends, relative to the superblock, for a file of + /// `file_len` bytes whose superblock is at `user_block` (both counted + /// from the start of the file), with libhdf5's truncation check + /// (`H5F__super_read`). + /// + /// The superblock records the end of the file's data as an absolute + /// address. A file shorter than that was truncated, and libhdf5 refuses + /// to open it ("truncated file"); so does this, with + /// [`FormatError::TruncatedFile`]. Bytes past that address are not part + /// of the file: libhdf5 fails any read of them ("addr overflow" / + /// "address plus size exceeds file eoa"), so a reader should parse only + /// the data up to the returned end. As libhdf5 does for a SWMR reader, + /// the check is skipped for a version-3 superblock whose writer is still + /// writing it in SWMR mode (it extends the file as it goes); the data + /// then ends at the end of the file. + /// + /// When the superblock's recorded base address differs from where the + /// superblock actually is (a user block added or removed after the file + /// was written), libhdf5 moves the recorded end of file by the same + /// amount, and so does this. + pub fn data_end(&self, user_block: u64, file_len: u64) -> Result { + let eof = + i128::from(self.eof_address) - i128::from(self.base_address) + i128::from(user_block); + if eof < 0 || eof > i128::from(file_len) { + if self.version >= 3 && self.is_swmr_write() { + return Ok(file_len.saturating_sub(user_block)); + } + return Err(FormatError::TruncatedFile { + stored_eof: u64::try_from(eof).unwrap_or(self.eof_address), + actual_len: file_len, + }); + } + // 0 <= eof <= file_len, so it fits a u64. + Ok((eof as u64).saturating_sub(user_block)) + } + /// Whether the file was opened with write access when the superblock was written. pub fn is_write_access(&self) -> bool { self.consistency_flags & swmr_flags::WRITE_ACCESS != 0 @@ -537,6 +573,30 @@ mod tests { buf } + #[test] + fn data_end_refuses_truncated_files_like_libhdf5() { + // build_v2_bytes records base 0, end of file 2048. + let sb = Superblock::parse(&build_v2_bytes(8, 2), 0).unwrap(); + assert_eq!(sb.data_end(0, 2048), Ok(2048)); + // Bytes past the recorded end are not part of the file. + assert_eq!(sb.data_end(0, 4096), Ok(2048)); + assert_eq!( + sb.data_end(0, 2047), + Err(FormatError::TruncatedFile { + stored_eof: 2048, + actual_len: 2047 + }) + ); + // A user block added in front after the file was written (the + // recorded base address is still 0): the end moves with it. + assert_eq!(sb.data_end(512, 2560), Ok(2048)); + assert!(sb.data_end(512, 2559).is_err()); + // A v3 superblock still being written in SWMR mode is not checked. + let mut swmr = Superblock::parse(&build_v2_bytes(8, 3), 0).unwrap(); + swmr.consistency_flags = swmr_flags::WRITE_ACCESS | swmr_flags::SWMR_WRITE; + assert_eq!(swmr.data_end(0, 1000), Ok(1000)); + } + #[test] fn parse_v0_8byte_offsets() { let data = build_v0_bytes(8); diff --git a/crates/clawhdf5-io/src/async_read.rs b/crates/clawhdf5-io/src/async_read.rs index d9e2ffe..30969fa 100644 --- a/crates/clawhdf5-io/src/async_read.rs +++ b/crates/clawhdf5-io/src/async_read.rs @@ -281,8 +281,13 @@ impl AsyncHDF5File { // HDF5 addresses are relative to the superblock: drop any user block // so they index `data` directly. let user_block = find_signature(&data)?; + let whole_len = data.len() as u64; data.drain(..user_block); let superblock = Superblock::parse(&data, 0)?; + // Refuse a truncated file, and keep nothing past the end of file the + // superblock records, as libhdf5 does. + let end = superblock.data_end(user_block as u64, whole_len)?; + data.truncate(end as usize); Ok(Self { data, superblock }) } @@ -312,7 +317,7 @@ impl AsyncHDF5File { let dt_msg = find_msg(&header, MessageType::Datatype).ok_or(FormatError::DatasetMissingData)?; - let (datatype, _) = Datatype::parse(&dt_msg.data)?; + let (datatype, _) = Datatype::parse_in_header(&dt_msg.data, header.version)?; let ds_msg = find_msg(&header, MessageType::Dataspace).ok_or(FormatError::DatasetMissingShape)?; @@ -605,6 +610,28 @@ mod tests { tokio::fs::remove_file(&path).await.ok(); } + /// As libhdf5 does: a truncated file is refused, and bytes past the end + /// of file the superblock records are dropped. + #[tokio::test] + async fn async_refuses_truncated_files_and_drops_trailing_bytes() { + let bytes = make_test_hdf5_f64("v", &[1.0, 2.0]); + let truncated = bytes[..bytes.len() - 8].to_vec(); + let err = AsyncHDF5File::from_bytes(truncated).err().unwrap(); + assert!( + matches!( + err, + AsyncHDF5Error::Format(FormatError::TruncatedFile { .. }) + ), + "{err}" + ); + + let mut appended = bytes.clone(); + appended.extend_from_slice(&[0xAB; 64]); + let file = AsyncHDF5File::from_bytes(appended).unwrap(); + assert_eq!(file.as_bytes().len(), bytes.len()); + assert_eq!(file.read_f64("v").await.unwrap(), vec![1.0, 2.0]); + } + #[tokio::test] async fn async_error_display() { let io_err = AsyncHDF5Error::Io(io::Error::new(io::ErrorKind::NotFound, "gone")); diff --git a/crates/clawhdf5-io/src/mpi_vol.rs b/crates/clawhdf5-io/src/mpi_vol.rs index 7dac39e..87a82ea 100644 --- a/crates/clawhdf5-io/src/mpi_vol.rs +++ b/crates/clawhdf5-io/src/mpi_vol.rs @@ -180,8 +180,7 @@ fn mpi_collective_read(vol: &MpiVol, location: &str, path: &str) -> Result Result Result, } +/// The HDF5 bytes of a whole file and its superblock: from the superblock +/// (addresses are relative to it, so any user block is skipped) to the end +/// of file the superblock records. A file shorter than that is truncated +/// and refused, and nothing past it is read, as in libhdf5. +pub(crate) fn hdf5_view( + whole: &[u8], +) -> Result<(&[u8], clawhdf5_format::superblock::Superblock), VolError> { + use clawhdf5_format::{signature::split_user_block, superblock::Superblock}; + let err = |e: clawhdf5_format::error::FormatError| VolError::DataError(e.to_string()); + let (user_block, data) = split_user_block(whole).map_err(err)?; + let sb = Superblock::parse(data, 0).map_err(err)?; + let end = sb + .data_end(user_block.len() as u64, whole.len() as u64) + .map_err(err)?; + // data_end is at most the file length less the user block. + Ok((&data[..end as usize], sb)) +} + impl NativeVol { /// Create a new native VOL connector. pub fn new() -> Self { @@ -264,6 +282,8 @@ impl VirtualObjectLayer for NativeVol { fn open(&mut self, location: &str) -> Result<(), VolError> { let data = std::fs::read(location)?; + // Refuse a truncated file at open, as libhdf5 does. + hdf5_view(&data)?; self.data = Some(data); self.location = Some(location.to_string()); Ok(()) @@ -283,13 +303,10 @@ impl VirtualObjectLayer for NativeVol { use clawhdf5_format::{ data_layout::DataLayout, data_read::read_raw_data_full, dataspace::Dataspace, datatype::Datatype, filter_pipeline::FilterPipeline, group_v2::resolve_path_any, - message_type::MessageType, object_header::ObjectHeader, signature::split_user_block, - superblock::Superblock, + message_type::MessageType, object_header::ObjectHeader, }; - // Addresses are relative to the superblock: skip any user block. - let (_, data) = split_user_block(data).map_err(|e| VolError::DataError(e.to_string()))?; - let sb = Superblock::parse(data, 0).map_err(|e| VolError::DataError(e.to_string()))?; + let (data, sb) = hdf5_view(data)?; let addr = resolve_path_any(data, &sb, path) .map_err(|e| VolError::NotFound(format!("{path}: {e}")))?; @@ -301,8 +318,8 @@ impl VirtualObjectLayer for NativeVol { .iter() .find(|m| m.msg_type == MessageType::Datatype) .ok_or_else(|| VolError::DataError("missing datatype".into()))?; - let (datatype, _) = - Datatype::parse(&dt_msg.data).map_err(|e| VolError::DataError(e.to_string()))?; + let (datatype, _) = Datatype::parse_in_header(&dt_msg.data, header.version) + .map_err(|e| VolError::DataError(e.to_string()))?; let ds_msg = header .messages @@ -387,6 +404,36 @@ mod tests { assert!(vol.as_bytes().is_none()); } + /// As libhdf5 does: a file shorter than the end of file its superblock + /// records is truncated and refused (at open, and when read from + /// memory), and bytes appended past that end are not part of the file. + #[test] + fn native_vol_refuses_truncated_files_and_ignores_trailing_bytes() { + use clawhdf5_format::file_writer::FileWriter as FmtWriter; + + let mut fw = FmtWriter::new(); + fw.create_dataset("x").with_f64_data(&[1.0, 2.0, 3.0]); + let bytes = fw.finish().unwrap(); + + let truncated = bytes[..bytes.len() - 8].to_vec(); + let err = NativeVol::from_bytes(truncated.clone()) + .read_dataset("x") + .unwrap_err(); + assert!(err.to_string().contains("truncated"), "{err}"); + let dir = std::env::temp_dir().join(format!("clawhdf5_vol_trunc_{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("truncated.h5"); + std::fs::write(&path, &truncated).unwrap(); + let err = NativeVol::open_path(path.to_str().unwrap()).err().unwrap(); + assert!(err.to_string().contains("truncated"), "{err}"); + std::fs::remove_dir_all(&dir).ok(); + + let mut appended = bytes.clone(); + appended.extend_from_slice(&[0xAB; 64]); + let raw = NativeVol::from_bytes(appended).read_dataset("x").unwrap(); + assert_eq!(raw.len(), 24); + } + #[test] fn vol_error_display() { let err = VolError::Unsupported("read_dataset".into()); diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index 75ff609..d893485 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -43,6 +43,8 @@ pub struct LazyFile { /// Offset of the superblock in the file (the user-block size); every /// HDF5 address is relative to it. base: usize, + /// End of the HDF5 data (`Superblock::data_end`, absolute). + end: usize, superblock: Superblock, root_header: ObjectHeader, /// Cache of parsed object headers, keyed by address. @@ -74,9 +76,13 @@ impl LazyFile { /// /// Parses only the superblock and root group object header. pub fn open(reader: R) -> Result { + let whole_len = reader.as_bytes().len() as u64; let (user_block, data) = signature::split_user_block(reader.as_bytes())?; let base = user_block.len(); let superblock = Superblock::parse(data, 0)?; + // Refuse a truncated file; read nothing past the recorded end of file. + let end = base + superblock.data_end(base as u64, whole_len)? as usize; + let data = &reader.as_bytes()[base..end]; let root_header = ObjectHeader::parse( data, superblock.root_group_address as usize, @@ -86,6 +92,7 @@ impl LazyFile { Ok(Self { reader, base, + end, superblock, root_header, header_cache: RefCell::new(HashMap::new()), @@ -104,7 +111,7 @@ impl LazyFile { } fn hdf5_bytes(&self) -> &[u8] { - &self.reader.as_bytes()[self.base..] + &self.reader.as_bytes()[self.base..self.end] } /// Returns a reference to the parsed superblock. @@ -479,7 +486,7 @@ impl<'f, R: HDF5Read> LazyDataset<'f, R> { fn datatype(&self) -> Result { let data = self.required_payload(MessageType::Datatype)?; - let (dt, _) = Datatype::parse(&data)?; + let (dt, _) = Datatype::parse_in_header(&data, self.header.version)?; Ok(dt) } diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index eeb1efb..7f544ca 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -35,6 +35,8 @@ pub struct MmapFile { /// Offset of the superblock in the mapped file (the user-block size); /// every HDF5 address is relative to it. base: usize, + /// End of the HDF5 data (`Superblock::data_end`, absolute). + end: usize, superblock: Superblock, } @@ -42,12 +44,16 @@ impl MmapFile { /// Open an HDF5 file using memory-mapped I/O. pub fn open>(path: P) -> Result { let reader = MmapReader::open(path).map_err(Error::Io)?; + let whole_len = reader.as_bytes().len() as u64; let (user_block, data) = signature::split_user_block(reader.as_bytes())?; let base = user_block.len(); let superblock = Superblock::parse(data, 0)?; + // Refuse a truncated file; read nothing past the recorded end of file. + let end = base + superblock.data_end(base as u64, whole_len)? as usize; Ok(Self { reader, base, + end, superblock, }) } @@ -55,7 +61,7 @@ impl MmapFile { /// The file's bytes from the superblock on — the space HDF5 addresses /// index into. fn hdf5_bytes(&self) -> &[u8] { - &self.reader.as_bytes()[self.base..] + &self.reader.as_bytes()[self.base..self.end] } /// Size of the user block before the superblock (0 for most files). @@ -426,7 +432,7 @@ impl<'f> MmapDataset<'f> { fn datatype(&self) -> Result { let data = self.required_payload(MessageType::Datatype)?; - let (dt, _) = Datatype::parse(&data)?; + let (dt, _) = Datatype::parse_in_header(&data, self.header.version)?; Ok(dt) } diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 65c0292..6e506b4 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -45,26 +45,34 @@ impl Backing { } } -/// The file's bytes, viewed from the superblock on. A file may start with a -/// user block (the superblock at 512, 1024, …); every HDF5 address is -/// relative to the superblock, so all parsing goes through [`Self::as_bytes`]. +/// The file's bytes, viewed from the superblock on and up to the end of +/// file the superblock records. A file may start with a user block (the +/// superblock at 512, 1024, …); every HDF5 address is relative to the +/// superblock, so all parsing goes through [`Self::as_bytes`]. struct FileData { backing: Backing, /// Offset of the superblock in the file (the user-block size). base: usize, + /// End of the HDF5 data in the file (`Superblock::data_end`, absolute). + end: usize, } impl FileData { - /// Locate the superblock and parse it. + /// Locate the superblock and parse it. A truncated file is refused, and + /// bytes past the recorded end of file are not read, as in libhdf5. fn new(backing: Backing) -> Result<(Self, Superblock), Error> { - let (user_block, hdf5) = signature::split_user_block(backing.whole_file())?; + let whole = backing.whole_file(); + let (user_block, hdf5) = signature::split_user_block(whole)?; let base = user_block.len(); let superblock = Superblock::parse(hdf5, 0)?; - Ok((Self { backing, base }, superblock)) + let end = superblock.data_end(base as u64, whole.len() as u64)?; + // data_end is at most the file length (less the user block). + let end = base + end as usize; + Ok((Self { backing, base, end }, superblock)) } fn as_bytes(&self) -> &[u8] { - &self.backing.whole_file()[self.base..] + &self.backing.whole_file()[self.base..self.end] } fn len(&self) -> usize { @@ -850,7 +858,7 @@ impl<'f> Dataset<'f> { fn datatype(&self) -> Result { let data = self.required_payload(MessageType::Datatype)?; - let (dt, _) = Datatype::parse(&data)?; + let (dt, _) = Datatype::parse_in_header(&data, self.header.version)?; Ok(dt) } diff --git a/crates/clawhdf5/tests/chunk_index_interop.rs b/crates/clawhdf5/tests/chunk_index_interop.rs index b72b7e2..3ff0420 100644 --- a/crates/clawhdf5/tests/chunk_index_interop.rs +++ b/crates/clawhdf5/tests/chunk_index_interop.rs @@ -515,6 +515,31 @@ fn we_write_maxshape_larger_than_shape() { check_we_write(&cases); } +/// A version-4 layout must encode every chunk dimension in the fewest bytes +/// that hold the largest one, as libhdf5 does: HDF5 2.0.0 (h5py 3.16) +/// refuses a wider encoding ("stored chunk dimension encoding length does +/// not match value calculated from chunk dimensions"). We rounded 3 bytes +/// up to 4, so h5py could not open any dataset we wrote with a chunk +/// dimension from 65 536 to 16 777 215. +#[test] +fn we_write_chunk_dimensions_in_the_fewest_bytes() { + const U: u64 = u64::MAX; + let mut cases = vec![ + // Single chunk, Fixed Array, Extensible Array, v2 B-tree. + wcase("single_70000", &[70_000], &[70_000], None), + wcase("fa_70000", &[140_000], &[70_000], None), + wcase("ea_70000", &[140_000], &[70_000], Some(&[U])), + wcase("bt2_70000", &[2, 70_000], &[1, 70_000], Some(&[U, U])), + // 2 bytes and 1 byte still, with the element size (4) the largest. + wcase("fa_300", &[600], &[300], None), + wcase("fa_3", &[6], &[3], None), + ]; + let mut filtered = wcase("single_70000_deflate", &[70_000], &[70_000], None); + filtered.deflate = true; + cases.push(filtered); + check_we_write(&cases); +} + /// More than one unlimited dimension needs a version-2 B-tree chunk index, /// as the library uses; an Extensible Array for `(None, None)` made libhdf5 /// refuse the whole file ("already found unlimited dimension"). diff --git a/crates/clawhdf5/tests/fixtures/written_by_v2_7_0.h5 b/crates/clawhdf5/tests/fixtures/written_by_v2_7_0.h5 new file mode 100644 index 0000000..ebb7cf1 Binary files /dev/null and b/crates/clawhdf5/tests/fixtures/written_by_v2_7_0.h5 differ diff --git a/crates/clawhdf5/tests/fixtures/written_by_v2_7_0_paged.h5 b/crates/clawhdf5/tests/fixtures/written_by_v2_7_0_paged.h5 new file mode 100644 index 0000000..8f83714 Binary files /dev/null and b/crates/clawhdf5/tests/fixtures/written_by_v2_7_0_paged.h5 differ diff --git a/crates/clawhdf5/tests/h5py_chunked_read_tests.rs b/crates/clawhdf5/tests/h5py_chunked_read_tests.rs index 2b9e902..40bfbf5 100644 --- a/crates/clawhdf5/tests/h5py_chunked_read_tests.rs +++ b/crates/clawhdf5/tests/h5py_chunked_read_tests.rs @@ -343,3 +343,41 @@ print("OK") let want: Vec = line[990..].iter().flat_map(|v| v.to_le_bytes()).collect(); assert_eq!(tail, want); } + +// --------------------------------------------------------------------------- +// Chunk dimension widths in layout version 4 +// --------------------------------------------------------------------------- + +/// A version-4 layout stores every chunk dimension in the fewest bytes that +/// hold the largest one (the element size included). A chunk dimension of +/// 70 000 takes 3 bytes and 2^32 + 1 elements would take 5; widths other +/// than 1, 2, 4 and 8 were refused, so these h5py files did not open. +#[test] +fn h5py_layout_v4_chunk_dimensions_of_3_bytes_read() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("enc3.h5"); + let p = path.display().to_string(); + run_python(&format!( + r#" +import h5py, numpy as np +with h5py.File("{p}", "w", libver="latest") as f: + f.create_dataset("single", data=np.arange(70000, dtype=" 0 or raw.find(bytes([4, 2, 1, 2, 3])) > 0 +"# + )); + let file = File::open(&path).unwrap(); + let single = file.dataset("single").unwrap().read_u64().unwrap(); + assert!(single.iter().copied().eq(0..70000), "single"); + let ea = file.dataset("ea").unwrap().read_f64().unwrap(); + assert_eq!(ea, (0..10).map(f64::from).collect::>()); + let fa = file.dataset("fa").unwrap().read_i32().unwrap(); + assert!(fa.iter().copied().eq(0..3 * 70000), "fa"); +} diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs new file mode 100644 index 0000000..2f52d27 --- /dev/null +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -0,0 +1,510 @@ +//! Corrupt files that libhdf5 refuses must be refused here too, not read. +//! +//! h5py writes a valid file, the script corrupts a copy the way a damaged or +//! malicious file would be, and records whether h5py (libhdf5) still opens +//! and reads the object. clawhdf5 must agree: read the valid file, refuse +//! each corrupt one. Skipped when python3 with h5py is unavailable, unless +//! `CLAWHDF5_REQUIRE_INTEROP=1`. + +use std::path::Path; +use std::process::Command; + +use clawhdf5::File; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn interop_required() -> bool { + std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1") +} + +fn python_available() -> bool { + Command::new(python()) + .args(["-c", "import h5py, numpy"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +macro_rules! skip_if_no_python { + () => { + if !python_available() { + assert!( + !interop_required(), + "CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available" + ); + eprintln!("SKIP: python3 with h5py not available"); + return; + } + }; +} + +/// Runs `body` (Python, with `h5py`, `numpy as np`, `struct` imported and +/// `d` the output directory) and then, for every `NAME.h5` it wrote, +/// prints `NAME ok` when h5py opens and reads dataset `d` and `NAME ERROR` +/// otherwise. Returns those lines, sorted. +fn h5py_verdicts(dir: &Path, body: &str) -> Vec { + let script = format!( + r#" +import h5py, numpy as np, struct, os, glob +d = "{dir}" +{body} +for path in sorted(glob.glob(os.path.join(d, "*.h5"))): + name = os.path.basename(path)[:-3] + try: + with h5py.File(path, "r") as f: + f["d"][()] + print(name, "ok") + except Exception: + print(name, "ERROR") +"#, + dir = dir.display() + ); + let out = Command::new(python()) + .args(["-c", &script]) + .output() + .expect("failed to run python"); + assert!( + out.status.success(), + "python failed:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + let mut lines: Vec = String::from_utf8_lossy(&out.stdout) + .lines() + .map(str::to_owned) + .collect(); + lines.sort(); + lines +} + +/// Whether clawhdf5 opens and reads dataset `d` of `path` (as raw bytes of +/// whatever type it has). +fn clawhdf5_reads(path: &Path) -> Result<(), String> { + let file = File::open(path).map_err(|e| format!("open: {e}"))?; + let ds = file.dataset("d").map_err(|e| format!("dataset: {e}"))?; + ds.dtype().map_err(|e| format!("dtype: {e}"))?; + ds.shape().map_err(|e| format!("shape: {e}"))?; + file.read_multi(&["d"]) + .map(|_| ()) + .map_err(|e| format!("read: {e}")) +} + +/// h5py's verdict for each file must be `expected`, and clawhdf5 must read +/// exactly the files h5py reads. +fn assert_agrees_with_h5py(dir: &Path, verdicts: &[String], expected: &[&str]) { + assert_eq!(verdicts, expected, "h5py's view changed"); + for line in verdicts { + let (name, verdict) = line.split_once(' ').unwrap(); + let ours = clawhdf5_reads(&dir.join(format!("{name}.h5"))); + match verdict { + "ok" => assert!(ours.is_ok(), "{name}: h5py reads it, we fail: {ours:?}"), + _ => assert!(ours.is_err(), "{name}: h5py refuses it, we read it"), + } + } +} + +#[test] +fn chunk_dimensions_libhdf5_refuses_are_refused() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + // A chunked int32 dataset (chunk 37, element size 4 after it in the + // layout message), with libver earliest (layout v3) and latest (v4). + // The corrupt copies set the chunk dimension to 0 (which read as all + // fill values), to 0x80000000 (an 8 GiB chunk) and to 38, which the + // chunk index's offsets 37 and 74 are not multiples of (libhdf5: "bad + // coordinate offset"; the chunks were read at the wrong place). A v4 + // layout indexes chunks by position, not offset, but both libraries + // refuse the changed chunk grid there too. + let verdicts = h5py_verdicts( + dir.path(), + r#" +for libver in ("earliest", "latest"): + good = os.path.join(d, f"{libver}_good.h5") + with h5py.File(good, "w", libver=libver) as f: + f.create_dataset("d", data=np.arange(100, dtype=" 0 + for name, value in (("zero", 0), ("huge", 0x80000000), ("offgrid", 38)): + bad = bytearray(data) + if value >= 1 << (8 * width): + continue + bad[at:at + width] = value.to_bytes(width, "little") + open(os.path.join(d, f"{libver}_{name}.h5"), "wb").write(bad) +"#, + ); + assert_agrees_with_h5py( + dir.path(), + &verdicts, + &[ + "earliest_good ok", + "earliest_huge ERROR", + "earliest_offgrid ERROR", + "earliest_zero ERROR", + "latest_good ok", + "latest_offgrid ERROR", + "latest_zero ERROR", + ], + ); +} + +/// Python: `fix_ohdr(buf, off)` recomputes the Jenkins lookup3 checksum of +/// the version-2 object header chunk 0 at `off`, so a field can be changed +/// in a `libver="latest"` file without the checksum failing first. +const FIX_OHDR_PY: &str = r#" +def _rot(x, k): + return ((x << k) | (x >> (32 - k))) & 0xFFFFFFFF +def lookup3(data): + M = 0xFFFFFFFF + n = len(data); a = b = c = (0xDEADBEEF + n) & M; i = 0 + w = lambda j: int.from_bytes(data[j:j + 4], "little") + while n > 12: + a = (a + w(i)) & M; b = (b + w(i + 4)) & M; c = (c + w(i + 8)) & M + a = (a - c) & M; a ^= _rot(c, 4); c = (c + b) & M + b = (b - a) & M; b ^= _rot(a, 6); a = (a + c) & M + c = (c - b) & M; c ^= _rot(b, 8); b = (b + a) & M + a = (a - c) & M; a ^= _rot(c, 16); c = (c + b) & M + b = (b - a) & M; b ^= _rot(a, 19); a = (a + c) & M + c = (c - b) & M; c ^= _rot(b, 4); b = (b + a) & M + n -= 12; i += 12 + if n == 0: + return c + t = bytes(data[i:]) + bytes(12) + w = lambda j: int.from_bytes(t[j:j + 4], "little") + a = (a + w(0)) & M; b = (b + w(4)) & M; c = (c + w(8)) & M + c ^= b; c = (c - _rot(b, 14)) & M + a ^= c; a = (a - _rot(c, 11)) & M + b ^= a; b = (b - _rot(a, 25)) & M + c ^= b; c = (c - _rot(b, 16)) & M + a ^= c; a = (a - _rot(c, 4)) & M + b ^= a; b = (b - _rot(a, 14)) & M + c ^= b; c = (c - _rot(b, 24)) & M + return c +def fix_ohdr(buf, off): + assert buf[off:off + 4] == b"OHDR" + flags = buf[off + 5]; p = off + 6 + if flags & 0x20: p += 16 + if flags & 0x10: p += 4 + width = 1 << (flags & 3) + end = p + width + int.from_bytes(buf[p:p + width], "little") + buf[end:end + 4] = lookup3(bytes(buf[off:end])).to_bytes(4, "little") +"#; + +/// A chunked layout records the element size as its last dimension; libhdf5 +/// refuses a dataset whose datatype has another size ("stored datatype size +/// in chunk layout does not match datatype description"). This read the +/// chunks laid out with the wrong element size. +#[test] +fn chunk_layout_element_size_must_match_the_datatype() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let body = format!( + "{FIX_OHDR_PY}{}", + r#" +for libver in ("earliest", "latest"): + good = os.path.join(d, f"{libver}_good.h5") + with h5py.File(good, "w", libver=libver) as f: + f.create_dataset("d", data=np.arange(100, dtype=" 6 + for size in (2, 8): + bad = bytearray(data) + bad[at:at + width] = size.to_bytes(width, "little") + if libver == "latest": + fix_ohdr(bad, bad.rfind(b"OHDR", 0, at)) + open(os.path.join(d, f"{libver}_size{size}.h5"), "wb").write(bad) +"# + ); + let verdicts = h5py_verdicts(dir.path(), &body); + assert_agrees_with_h5py( + dir.path(), + &verdicts, + &[ + "earliest_good ok", + "earliest_size2 ERROR", + "earliest_size8 ERROR", + "latest_good ok", + "latest_size2 ERROR", + "latest_size8 ERROR", + ], + ); + for name in ["earliest_size2", "latest_size8"] { + let err = clawhdf5_reads(&dir.path().join(format!("{name}.h5"))).unwrap_err(); + assert!( + err.contains("stored datatype size in chunk layout"), + "{name}: {err}" + ); + } +} + +/// A v2 object header message whose size runs past the messages into the +/// chunk's checksum is refused by libhdf5 whether it runs 1 byte or more +/// into the checksum (its loop stops at the checksum, and then the checksum +/// read and the size check fail), so it is refused here too. +#[test] +fn v2_header_message_running_into_the_checksum_is_refused() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let body = format!( + "{FIX_OHDR_PY}{}", + r#" +good = os.path.join(d, "good.h5") +with h5py.File(good, "w", libver="latest") as f: + f.create_dataset("d", data=np.arange(4, dtype=" 0 +bad = bytearray(data); bad[at + 4] |= 0x40 +save("layout_shareable", bad) +# A message size that is not a multiple of 8 in a v1 header. +bad = bytearray(data); bad[at + 2] = 23 +save("layout_unaligned", bad) + +# A compound whose second field repeats the first's name, or overlaps it. +dt = np.dtype([("aa", ">()); +} + +/// A variable-length compound member takes 4 + offset size + 4 bytes, which +/// is 12 in a file with 4-byte offsets, and libhdf5 checks for overlapping +/// members with that stored size. A member right after one was refused as +/// "member overlaps with previous member" (the check took the 16 bytes of +/// an 8-byte-offset file), and with it every attribute of the object. +#[test] +fn variable_length_compound_members_in_files_with_4_byte_offsets() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + run_python( + dir.path(), + r#" +from h5py import h5f, h5p +dt = np.dtype([("s", h5py.string_dtype()), ("i", " PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures") + .join(name) +} + +#[test] +fn every_object_of_a_v2_7_0_file_reads() { + let file = File::open(fixture("written_by_v2_7_0.h5")).unwrap(); + + let (attrs, errors) = file.root().attrs_with_errors().unwrap(); + assert!(errors.is_empty(), "{errors:?}"); + // (It reads as no strings, as it did before.) + assert!( + matches!(&attrs["empty"], AttrValue::StringArray(v) if v.iter().all(String::is_empty)), + "{:?}", + attrs["empty"] + ); + assert!(matches!(&attrs["title"], AttrValue::String(s) if s == "old")); + + let f32s = |name: &str| file.dataset(name).unwrap().read_f32().unwrap(); + assert_eq!(f32s("f32"), [1.0, 2.0, 3.0]); + assert_eq!( + f32s("f32_2d"), + (0..60).map(|x| x as f32).collect::>() + ); + assert_eq!( + f32s("chunked"), + (0..1000).map(|x| x as f32).collect::>() + ); + assert!(f32s("empty").is_empty()); + assert_eq!( + file.dataset("f64").unwrap().read_f64().unwrap(), + [1.0, 2.0, 3.0] + ); + assert_eq!(file.dataset("i32").unwrap().read_i32().unwrap(), [1, -2, 3]); + assert_eq!(file.dataset("i64").unwrap().read_i64().unwrap(), [1, -2, 3]); + assert_eq!(file.dataset("u64").unwrap().read_u64().unwrap(), [1, 2, 3]); + assert_eq!( + file.dataset("chunked_2d").unwrap().read_i32().unwrap(), + (0..600).collect::>() + ); + for name in ["unlimited", "maxshape"] { + assert_eq!( + file.dataset(name).unwrap().read_f64().unwrap(), + (0..100).map(|x| x as f64).collect::>(), + "{name}" + ); + } + assert_eq!( + file.dataset("compact").unwrap().read_i32().unwrap(), + [7, 8, 9] + ); + for name in ["u8", "compound", "enum", "enum8"] { + file.dataset(name) + .unwrap() + .dtype() + .unwrap_or_else(|e| panic!("{name}: {e}")); + } + + let grp = file.group("grp").unwrap(); + let (attrs, errors) = grp.attrs_with_errors().unwrap(); + assert!(errors.is_empty(), "{errors:?}"); + assert_eq!(attrs.len(), 21); + assert_eq!(grp.dataset("d").unwrap().read_f32().unwrap(), [4.0, 5.0]); + + let paged = File::open(fixture("written_by_v2_7_0_paged.h5")).unwrap(); + assert_eq!(paged.dataset("d").unwrap().read_f32().unwrap(), [1.0, 2.0]); +} + +/// Datatypes libhdf5 (and this reader) refuse — a compound with a repeated +/// field name or no fields, an enum member with an empty name — must not be +/// written: they made files that did not read back. Valid neighbours of each +/// still write and read back. +#[test] +fn datatypes_the_reader_refuses_are_not_written() { + use clawhdf5::{CompoundTypeBuilder, EnumTypeBuilder, FileBuilder}; + + let write_compound = |dt, raw: Vec| { + let mut b = FileBuilder::new(); + b.create_dataset("d").with_compound_data(dt, raw, 1); + b.finish() + }; + let write_enum = |dt| { + let mut b = FileBuilder::new(); + b.create_dataset("d").with_enum_u8_data(dt, &[0, 1]); + b.finish() + }; + let refused = |r: Result, clawhdf5::Error>, what: &str| { + let err = r.expect_err("written"); + let msg = err.to_string(); + assert!(msg.contains("datatype cannot be written"), "{msg}"); + assert!(msg.contains(what), "{msg}"); + }; + + let dup = CompoundTypeBuilder::new() + .f64_field("x") + .f64_field("x") + .build(); + refused( + write_compound(dup, vec![0; 16]), + "duplicated compound field name 'x'", + ); + refused( + write_compound(CompoundTypeBuilder::new().build(), vec![]), + "invalid", + ); + let empty_name = EnumTypeBuilder::u8_based() + .u8_value("A", 0) + .u8_value("", 1) + .build(); + refused(write_enum(empty_name), "0 length enum name"); + + let ok = CompoundTypeBuilder::new() + .f64_field("x") + .f64_field("y") + .build(); + let bytes = write_compound(ok, vec![0; 16]).unwrap(); + let file = File::from_bytes(bytes).unwrap(); + file.dataset("d").unwrap().dtype().unwrap(); + let ok = EnumTypeBuilder::u8_based() + .u8_value("A", 0) + .u8_value("B", 1) + .build(); + let file = File::from_bytes(write_enum(ok).unwrap()).unwrap(); + file.dataset("d").unwrap().dtype().unwrap(); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index 4ea7518..617784a 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -129,13 +129,46 @@ fill-value item that did is fixed). failing the others. - **Other readers:** - VL-string datasets are not readable through `File`. + - Variable-length values inside a compound (and VL-string attributes) in + a file with 4-byte offsets (`sizeof_addr = 4`) fail with + `GlobalHeapObjectNotFound` or come back as `Raw`: these paths assume + the 16-byte element of an 8-byte-offset file. The datatype itself reads + (it was refused as "member overlaps with previous member" until + 2026-09-26). - Metadata cache images are not supported. - x87 long double and binary128 are refused. - N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail. - **Filters:** blosc, blosc2, bitshuffle, bzip2, LZF and zfp are not implemented. -- **Header checks:** on 12 CVE datasets libhdf5 rejects a corrupt header and - we read data anyway. We need stricter header checks. +- ~~**Header checks:** on 12 CVE datasets libhdf5 rejects a corrupt header and + we read data anyway. We need stricter header checks.~~ **Fixed + 2026-09-26** (counted again: 18 objects on the CVE corpus that libhdf5 + refuses; some read as wrong data, e.g. a zero chunk dimension read as all + fill values): object headers, datatypes, chunk dimensions and chunk-index + offsets are checked as libhdf5 checks them, and truncated files are + refused. 17 of the 18 now + fail as in libhdf5 (conformance on tank, `conformance/run.sh --no-fetch`, + 2026-09-26: 571 of 697 ok). Still read where libhdf5 refuses: + - `cve-2024-32624.h5` `/Dset_OBJREF`: a dataspace whose storage size + overflows 64 bits. `File::dataset` and `shape()` succeed (libhdf5 + refuses at open); reading the values fails. + - `cve-2020-10810.h5`, `cve-2020-10812.h5` (whole files libhdf5 cannot + open, not among the 18): libhdf5 decodes the superblock extension's File + Space Info and metadata-cache-image messages at open and refuses these + files; we do not decode those messages at open. + - Deliberately not refused, because clawhdf5 up to v2.7.0 wrote them: a + float sign bit position outside the type, and a size-0 string type. + - Not refused because current libhdf5 reads it though HDF5 2.0.0 + (h5py 3.16) refuses it: a v4 chunked layout whose dimensions are + encoded in more bytes than they need (HDFGroup/hdf5@e124c36, + 2026-06-05, relaxed that check; clawhdf5 wrote such layouts until + 2026-09-26). + - Not refused because HDF5 2.0 (h5py 3.16) reads them though newer + libhdf5 refuses them: bit-field offset/precision outside the type, an + unknown variable-length kind, an array type whose stored size is not + its element count times its base size. + - (`cve-2024-32616` `/group1/dset3` and `cve-2025-2309`'s `Comp_OBJREF` + attribute are h5py/numpy type-mapping failures, not libhdf5 refusals.) - **Writer:** - Nested groups beyond one level: path-like names are now refused, not created.