fix: refuse at open the dataset storage libhdf5 refuses at open
libhdf5 checks a dataset's storage when it opens the dataset (H5D__contig_check, H5D__compact_init): the element count times the element size must not overflow, contiguous storage must end inside the file, compact data must be the dataset's size. File::dataset opened cve-2024-32624's /Dset_OBJREF (2^62 + 2 references of 8 bytes) and reported its shape; only reading failed. data_read::check_dataset_storage makes those checks (new FormatError::InvalidDatasetStorage), and File, MmapFile and LazyFile run it whenever they open a dataset (by path, by address, from a group), as does the conformance probe. As before, a datatype, dataspace or layout that does not decode is left for the read to report, so such a dataset still opens and its attributes still read. An empty contiguous dataset at a defined address, which libhdf5 refuses, is still accepted: clawhdf5 up to v2.7.0 wrote them. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -32,6 +32,64 @@ fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatErr
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The storage checks libhdf5 makes when it opens a dataset, before any
|
||||
/// data is read (`H5D__contig_check`, `H5D__compact_init`), so a dataset
|
||||
/// they refuse fails to open, as in libhdf5, instead of opening and
|
||||
/// reporting a shape nothing can be read from:
|
||||
///
|
||||
/// - the element count times the element size must not overflow 64 bits
|
||||
/// ("size of dataset's storage overflowed" — `cve-2024-32624`
|
||||
/// `/Dset_OBJREF`, 2^62 references of 8 bytes);
|
||||
/// - contiguous storage at a defined address must end within the file's
|
||||
/// `file_len` bytes (the HDF5 data up to the end of file the superblock
|
||||
/// records);
|
||||
/// - compact data must be exactly the dataset's size.
|
||||
///
|
||||
/// Deliberately not refused, unlike libhdf5: an empty contiguous dataset at
|
||||
/// a defined address (libhdf5's overflow test `addr + 0 <= addr` refuses
|
||||
/// it), which clawhdf5 up to v2.7.0 wrote. Chunked and virtual layouts are
|
||||
/// checked when their data is read.
|
||||
pub fn check_dataset_storage(
|
||||
layout: &DataLayout,
|
||||
dataspace: &Dataspace,
|
||||
datatype: &Datatype,
|
||||
file_len: u64,
|
||||
) -> Result<(), FormatError> {
|
||||
if !matches!(
|
||||
layout,
|
||||
DataLayout::Contiguous { .. } | DataLayout::Compact { .. }
|
||||
) {
|
||||
return Ok(());
|
||||
}
|
||||
const OVERFLOWED: &str = "size of dataset's storage overflowed";
|
||||
let n = dataspace
|
||||
.checked_num_elements()
|
||||
.map_err(|_| FormatError::InvalidDatasetStorage(OVERFLOWED))?;
|
||||
let data_size = n
|
||||
.checked_mul(u64::from(datatype.type_size()))
|
||||
.ok_or(FormatError::InvalidDatasetStorage(OVERFLOWED))?;
|
||||
match layout {
|
||||
DataLayout::Contiguous {
|
||||
address: Some(address),
|
||||
..
|
||||
} if address
|
||||
.checked_add(data_size)
|
||||
.is_none_or(|end| end > file_len) =>
|
||||
{
|
||||
Err(FormatError::InvalidDatasetStorage(
|
||||
"invalid dataset size, likely file corruption",
|
||||
))
|
||||
}
|
||||
DataLayout::Compact { data } if data.len() as u64 != data_size => {
|
||||
Err(FormatError::InvalidDatasetStorage(
|
||||
"bad value from dataset header - size of compact dataset's data buffer \
|
||||
doesn't match size of dataset data",
|
||||
))
|
||||
}
|
||||
_ => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// How many bytes to read from a contiguous dataset's storage of
|
||||
/// `storage_size` bytes (the layout message's size) holding `needed` bytes
|
||||
/// of elements. libhdf5 reads the elements' bytes from the start of the
|
||||
@@ -2666,6 +2724,41 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
/// `H5D__contig_check` / `H5D__compact_init`, run when a dataset opens.
|
||||
#[test]
|
||||
fn dataset_storage_checks_at_open() {
|
||||
let dt = make_f64_le_type();
|
||||
let contiguous = |address| DataLayout::Contiguous { address, size: 0 };
|
||||
// cve-2024-32624 `/Dset_OBJREF`: 2^62 + 2 elements of 8 bytes.
|
||||
let huge = make_simple_dataspace(&[(1 << 62) + 2]);
|
||||
assert_eq!(
|
||||
check_dataset_storage(&contiguous(None), &huge, &dt, 1 << 20),
|
||||
Err(FormatError::InvalidDatasetStorage(
|
||||
"size of dataset's storage overflowed"
|
||||
))
|
||||
);
|
||||
let ds = make_simple_dataspace(&[4]);
|
||||
assert!(check_dataset_storage(&contiguous(Some(100)), &ds, &dt, 132).is_ok());
|
||||
assert!(matches!(
|
||||
check_dataset_storage(&contiguous(Some(100)), &ds, &dt, 131),
|
||||
Err(FormatError::InvalidDatasetStorage(_))
|
||||
));
|
||||
assert!(matches!(
|
||||
check_dataset_storage(&contiguous(Some(u64::MAX - 8)), &ds, &dt, u64::MAX),
|
||||
Err(FormatError::InvalidDatasetStorage(_))
|
||||
));
|
||||
// Not allocated, and (unlike libhdf5) empty at a defined address.
|
||||
assert!(check_dataset_storage(&contiguous(None), &ds, &dt, 0).is_ok());
|
||||
let empty = make_simple_dataspace(&[0]);
|
||||
assert!(check_dataset_storage(&contiguous(Some(64)), &empty, &dt, 64).is_ok());
|
||||
let compact = |n: usize| DataLayout::Compact { data: vec![0; n] };
|
||||
assert!(check_dataset_storage(&compact(32), &ds, &dt, 0).is_ok());
|
||||
assert!(matches!(
|
||||
check_dataset_storage(&compact(24), &ds, &dt, 0),
|
||||
Err(FormatError::InvalidDatasetStorage(_))
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn zerocopy_size_mismatch() {
|
||||
let dt = make_f64_le_type();
|
||||
|
||||
@@ -230,6 +230,11 @@ pub enum FormatError {
|
||||
/// libhdf5's own error text): more than 32 dimensions, a rank on a
|
||||
/// scalar or null dataspace, a dimension larger than its maximum.
|
||||
InvalidDataspace(&'static str),
|
||||
/// A dataset whose storage libhdf5 refuses when it opens the dataset
|
||||
/// (the reason is libhdf5's own error text): an element count times
|
||||
/// element size that overflows, contiguous storage past the end of the
|
||||
/// file, compact data of the wrong size.
|
||||
InvalidDatasetStorage(&'static str),
|
||||
}
|
||||
|
||||
impl fmt::Display for FormatError {
|
||||
@@ -507,6 +512,9 @@ impl fmt::Display for FormatError {
|
||||
FormatError::InvalidDataspace(why) => {
|
||||
write!(f, "invalid dataspace: {why}")
|
||||
}
|
||||
FormatError::InvalidDatasetStorage(why) => {
|
||||
write!(f, "invalid dataset storage: {why}")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user