From 3cf8cd86f22bfbcca713443c41f177848643c7a1 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 00:08:58 -0500 Subject: [PATCH] fix(format): refuse datatypes libhdf5 refuses to decode Datatype::parse now makes the checks of libhdf5's H5O__dtype_decode_helper and fails with InvalidDatatype (libhdf5's own error text) instead of decoding a corrupt type: - size 0 ("invalid datatype size"), for every class; - integer bit offset/precision outside the type, or precision 0; - float sign/exponent/mantissa outside the type, empty, or overlapping; normalization 3; bit 6 without bit 0 from version 3; - compound with no members, a member outside the compound, a duplicate name, or a member overlapping an earlier one; - enum whose size differs from its base type's, or an empty member name; - array of more than 32 dimensions or with a zero-sized one (v1 compound array members now say so rather than InvalidDatatypeVersion); - opaque tag length that is not a multiple of 8. Bit 6 of a version-1/2 float's class bits used to be read as VAX order, byte-swapping values; libhdf5 ignores it before version 3, and so does this now. Only checks HDF5 2.0 (h5py 3.16) makes are added: newer libhdf5 also checks bit fields, the variable-length kind and array sizes, but h5py opens files that fail those, so they are left out. Each check was confirmed against h5py by corrupting a file it wrote. The conformance probe now decodes committed datatypes, as h5py's f[name] does. Conformance (cached corpus, tank): 570 ok, unchanged. Objects libhdf5 refuses that clawhdf5 used to read: cve-2016-4332-mtime (/cmpnd), cve-2017-17508, cve-2024-32616 (/type1), cve-2024-32618, cve-2026-34734, bad_compound.h5 (/cmpnd, /dataset); eight more that already failed now fail with libhdf5's reason (e.g. cve-2024-29163 "mantissa range out of bounds"). Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/probe/src/main.rs | 15 + crates/clawhdf5-format/src/attribute.rs | 3 +- crates/clawhdf5-format/src/datatype.rs | 461 +++++++++++++++++++++++- crates/clawhdf5-format/src/error.rs | 7 + 4 files changed, 473 insertions(+), 13 deletions(-) diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 1298cf0..c4cc012 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -309,6 +309,14 @@ 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(&dtb).map_err(e)?; + Ok(()) + } + fn read_dataset(&self, h: &ObjectHeader, rec: &mut Map) -> Result<(), String> { let dtb = self .payload(h, MessageType::Datatype)? @@ -778,6 +786,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..df17f74 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -573,7 +573,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/datatype.rs b/crates/clawhdf5-format/src/datatype.rs index ba85afa..efc2afb 100644 --- a/crates/clawhdf5-format/src/datatype.rs +++ b/crates/clawhdf5-format/src/datatype.rs @@ -208,6 +208,36 @@ 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])) +} + +/// A compound member's size in the compound, as libhdf5 counts it. +fn member_size(dt: &Datatype) -> u64 { + u64::from(dt.type_size()) +} + +/// 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 +262,101 @@ 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: sign, exponent and +/// mantissa must lie inside the type, be non-empty, and not overlap. +/// (libhdf5 does not check a float's bit offset and precision.) +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 sign >= bits { + return Err(invalid("sign bit position out of bounds")); + } + 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 +380,9 @@ impl Datatype { let size = LittleEndian::read_u32(&data[4..8]); let mut pos = 8; + if size == 0 { + return Err(invalid("invalid datatype size")); + } match class_id { 0 => { @@ -272,6 +396,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 +414,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 +438,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 +514,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 +531,40 @@ 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); + // 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], + 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().any(|m| { + let m_end = m.byte_offset + member_size(&m.datatype); + byte_offset < m_end && 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 +576,17 @@ 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; + check_member( + &members, + &name, + byte_offset, + stored_size, + member_size(&member_dt), + )?; members.push(CompoundMember { name, byte_offset, @@ -438,11 +626,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,6 +640,7 @@ 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; @@ -461,6 +650,13 @@ impl Datatype { dimensions: array_dims, }; } + check_member( + &members, + &name, + byte_offset, + stored_size, + member_size(&member_dt), + )?; members.push(CompoundMember { name, byte_offset, @@ -499,6 +695,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 +705,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 +768,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 +776,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 +793,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 +801,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(( @@ -985,7 +1191,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 +1219,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 +1233,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 +1383,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 +2256,234 @@ 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"); + assert_eq!( + invalid_reason(&build_dt_header(3, 1, [0, 0, 0], 0)), + "invalid datatype size" + ); + 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 [ + ((32, 23, 8, 0, 23), "sign bit position out of bounds"), + ((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 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/error.rs b/crates/clawhdf5-format/src/error.rs index 6d038f5..46f8123 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -206,6 +206,10 @@ pub enum FormatError { /// 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), } impl fmt::Display for FormatError { @@ -453,6 +457,9 @@ impl fmt::Display for FormatError { FormatError::InvalidObjectHeader(why) => { write!(f, "corrupt object header: {why}") } + FormatError::InvalidDatatype(why) => { + write!(f, "invalid datatype: {why}") + } } } }