Fix silent wrong data and libhdf5 interop found by the HDF5 audit #11

Merged
osobh merged 41 commits from fix/phase0-correctness into main 2026-09-26 02:42:54 +00:00
4 changed files with 254 additions and 24 deletions
Showing only changes of commit aef8e766ae - Show all commits
+199 -24
View File
@@ -69,7 +69,7 @@ pub fn compress_chunk(
let level = filter.client_data.first().copied().unwrap_or(6);
deflate_compress(&result, level)?
}
FILTER_LZ4 => lz4_compress(&result)?,
FILTER_LZ4 => lz4_compress(&result, &filter.client_data)?,
FILTER_ZSTD => {
let level = filter.client_data.first().copied().unwrap_or(3);
zstd_compress(&result, level)?
@@ -813,12 +813,32 @@ fn deflate_compress(_data: &[u8], _level: u32) -> Result<Vec<u8>, FormatError> {
Err(FormatError::UnsupportedFilter(FILTER_DEFLATE))
}
/// Decompress LZ4 data. Format: 4 bytes LE original size + LZ4 block data.
/// Default LZ4 block size of the registered HDF5 LZ4 filter (`H5Zlz4.c`,
/// `DEFAULT_BLOCK_SIZE`): 1 GiB, so an HDF5 chunk is normally one block.
#[cfg(feature = "lz4")]
const LZ4_DEFAULT_BLOCK_SIZE: usize = 1 << 30;
/// Decompress an LZ4 (filter 32004) chunk.
///
/// The 4-byte "original size" header is part of the attacker-controlled
/// compressed payload itself, so it is bounded against `expected_bytes` (the
/// pipeline's declared chunk size) before being used to size the output
/// allocation — otherwise a crafted 4-byte value can request up to ~4 GiB.
/// Two framings are read:
///
/// * The registered HDF5 LZ4 filter format (`H5Zlz4.c`, what libhdf5 +
/// hdf5plugin write, and what clawhdf5 writes after 2.7.0): an 8-byte
/// big-endian total decompressed size, a 4-byte big-endian block size, then
/// per block a 4-byte big-endian compressed length followed by the block. A
/// block whose compressed length equals its decompressed length is stored
/// raw.
/// * The legacy clawhdf5 framing (up to 2.7.0): a 4-byte little-endian size
/// followed by one raw LZ4 block. libhdf5 cannot read it.
///
/// They are told apart unambiguously: an HDF5 chunk is smaller than 4 GiB, so
/// the registered format's big-endian `u64` size always starts with four zero
/// bytes and the whole chunk is at least 12 bytes; a legacy chunk starts with
/// four zero bytes only when it is empty, and is then 5 bytes long.
///
/// Every size read from the payload is bounded against `expected_bytes` (the
/// pipeline's declared chunk size) before it sizes an allocation, so a crafted
/// header cannot request gigabytes.
#[cfg(feature = "lz4")]
fn lz4_decompress(data: &[u8], expected_bytes: usize) -> Result<Vec<u8>, FormatError> {
if data.len() < 4 {
@@ -826,38 +846,112 @@ fn lz4_decompress(data: &[u8], expected_bytes: usize) -> Result<Vec<u8>, FormatE
"lz4: data too short".into(),
));
}
let check_size = |size: usize| -> Result<(), FormatError> {
if expected_bytes != 0 && size > expected_bytes {
return Err(FormatError::DecompressionError(
"lz4: declared size exceeds chunk size".into(),
));
}
if size > MAX_DECOMPRESS_SIZE {
return Err(FormatError::DecompressionError(
"lz4: declared size exceeds limit".into(),
));
}
Ok(())
};
if data.len() >= 12 && data[..4] == [0, 0, 0, 0] {
return lz4_decompress_hdf5(data, check_size);
}
// Legacy clawhdf5 framing: 4-byte LE size + one LZ4 block.
let orig_size = u32::from_le_bytes([data[0], data[1], data[2], data[3]]) as usize;
if expected_bytes != 0 && orig_size > expected_bytes {
return Err(FormatError::DecompressionError(
"lz4: declared size exceeds chunk size".into(),
));
}
if orig_size > MAX_DECOMPRESS_SIZE {
return Err(FormatError::DecompressionError(
"lz4: declared size exceeds limit".into(),
));
}
check_size(orig_size)?;
lz4_flex::block::decompress(&data[4..], orig_size)
.map_err(|e| FormatError::DecompressionError(format!("lz4: {e}")))
}
/// Decode the registered HDF5 LZ4 framing (see [`lz4_decompress`]).
#[cfg(feature = "lz4")]
fn lz4_decompress_hdf5(
data: &[u8],
check_size: impl Fn(usize) -> Result<(), FormatError>,
) -> Result<Vec<u8>, FormatError> {
let err = |m: &str| FormatError::DecompressionError(format!("lz4: {m}"));
let be32 = |b: &[u8]| u32::from_be_bytes([b[0], b[1], b[2], b[3]]) as usize;
// The first four bytes are zero (checked by the caller), so the size is
// the low 32 bits of the big-endian u64.
let orig_size = be32(&data[4..8]);
check_size(orig_size)?;
let block_size = be32(&data[8..12]).min(orig_size);
if block_size == 0 && orig_size != 0 {
return Err(err("zero block size"));
}
let mut out = vec![0u8; orig_size];
let mut pos = 12usize;
let mut done = 0usize;
while done < orig_size {
let this_block = block_size.min(orig_size - done);
let comp_len = be32(
data.get(pos..pos + 4)
.ok_or_else(|| err("truncated block header"))?,
);
pos += 4;
let block = data
.get(pos..pos.saturating_add(comp_len))
.ok_or_else(|| err("truncated block"))?;
let dst = &mut out[done..done + this_block];
if comp_len == this_block {
dst.copy_from_slice(block);
} else {
let n = lz4_flex::block::decompress_into(block, dst)
.map_err(|e| FormatError::DecompressionError(format!("lz4: {e}")))?;
if n != this_block {
return Err(err("block decompressed to the wrong size"));
}
}
pos += comp_len;
done += this_block;
}
Ok(out)
}
#[cfg(not(feature = "lz4"))]
fn lz4_decompress(_data: &[u8], _expected_bytes: usize) -> Result<Vec<u8>, FormatError> {
Err(FormatError::UnsupportedFilter(FILTER_LZ4))
}
/// Compress data with LZ4 block format. Format: 4 bytes LE original size + LZ4 block data.
/// Compress data in the registered HDF5 LZ4 filter format (see
/// [`lz4_decompress`]), so libhdf5 with the LZ4 plugin (e.g. hdf5plugin) can
/// read it. `cd[0]`, when present and non-zero, is the block size in bytes,
/// as in `H5Zlz4.c`; otherwise the 1 GiB default applies.
#[cfg(feature = "lz4")]
fn lz4_compress(data: &[u8]) -> Result<Vec<u8>, FormatError> {
let compressed = lz4_flex::block::compress(data);
let mut result = Vec::with_capacity(4 + compressed.len());
result.extend_from_slice(&(data.len() as u32).to_le_bytes());
result.extend_from_slice(&compressed);
fn lz4_compress(data: &[u8], cd: &[u32]) -> Result<Vec<u8>, FormatError> {
let block_size = match cd.first() {
Some(&b) if b != 0 => b as usize,
_ => LZ4_DEFAULT_BLOCK_SIZE,
}
.min(data.len());
let mut result = Vec::with_capacity(16 + data.len() / 2);
result.extend_from_slice(&(data.len() as u64).to_be_bytes());
result.extend_from_slice(&(block_size as u32).to_be_bytes());
if block_size == 0 {
return Ok(result);
}
for block in data.chunks(block_size) {
let compressed = lz4_flex::block::compress(block);
if compressed.len() >= block.len() {
// Incompressible: stored raw, marked by length == block length.
result.extend_from_slice(&(block.len() as u32).to_be_bytes());
result.extend_from_slice(block);
} else {
result.extend_from_slice(&(compressed.len() as u32).to_be_bytes());
result.extend_from_slice(&compressed);
}
}
Ok(result)
}
#[cfg(not(feature = "lz4"))]
fn lz4_compress(_data: &[u8]) -> Result<Vec<u8>, FormatError> {
fn lz4_compress(_data: &[u8], _cd: &[u32]) -> Result<Vec<u8>, FormatError> {
Err(FormatError::UnsupportedFilter(FILTER_LZ4))
}
@@ -1480,11 +1574,92 @@ mod tests {
// --- LZ4 tests ---
/// Chunk (0,0) of `/DS1` in the HDF Group's `h5ex_d_lz4.h5` example,
/// written by libhdf5's registered LZ4 plugin with a 3-byte block size
/// (so it has many blocks, some stored raw). Byte range from h5py's
/// `get_chunk_info`; values are `i*j - j` (i32 LE), as h5py reads them.
#[test]
#[cfg(feature = "lz4")]
fn lz4_reads_registered_hdf5_format() {
let file: &[u8] = include_bytes!("../tests/fixtures/filters/h5ex_d_lz4.h5");
let chunk = &file[4016..4016 + 312];
let pipeline = FilterPipeline {
version: 2,
filters: vec![FilterDescription {
filter_id: FILTER_LZ4,
name: None,
flags: 1,
client_data: vec![3],
}],
};
let out = decompress_chunk(chunk, &pipeline, 4 * 8 * 4, 4).unwrap();
let expected: Vec<u8> = (0..4i32)
.flat_map(|i| (0..8i32).map(move |j| i * j - j))
.flat_map(i32::to_le_bytes)
.collect();
assert_eq!(out, expected);
}
#[test]
#[cfg(feature = "lz4")]
fn lz4_writes_registered_hdf5_format() {
// Compressible data, one block: 8-byte BE size, 4-byte BE block size,
// 4-byte BE compressed length, block.
let data = vec![7u8; 1000];
let c = lz4_compress(&data, &[]).unwrap();
assert_eq!(&c[0..8], &1000u64.to_be_bytes());
assert_eq!(&c[8..12], &1000u32.to_be_bytes());
let len = u32::from_be_bytes(c[12..16].try_into().unwrap()) as usize;
assert_eq!(c.len(), 16 + len);
assert!(len < 1000);
assert_eq!(lz4_decompress(&c, 1000).unwrap(), data);
// Several blocks, incompressible ones stored raw (length == block).
let data: Vec<u8> = (0..10u8).collect();
let c = lz4_compress(&data, &[3]).unwrap();
assert_eq!(&c[8..12], &3u32.to_be_bytes());
assert_eq!(&c[12..16], &3u32.to_be_bytes());
assert_eq!(&c[16..19], &[0, 1, 2]);
assert_eq!(c.len(), 12 + 3 * (4 + 3) + (4 + 1));
assert_eq!(lz4_decompress(&c, 10).unwrap(), data);
let c = lz4_compress(&[], &[]).unwrap();
assert_eq!(lz4_decompress(&c, 0).unwrap(), Vec::<u8>::new());
}
/// Chunks written by clawhdf5 up to 2.7.0 (4-byte LE size + one LZ4
/// block) must stay readable.
#[test]
#[cfg(feature = "lz4")]
fn lz4_reads_legacy_clawhdf5_format() {
for data in [vec![], vec![5u8; 300], (0..=255u8).collect::<Vec<u8>>()] {
let mut legacy = (data.len() as u32).to_le_bytes().to_vec();
legacy.extend_from_slice(&lz4_flex::block::compress(&data));
assert_eq!(lz4_decompress(&legacy, data.len()).unwrap(), data);
}
}
#[test]
#[cfg(feature = "lz4")]
fn lz4_registered_format_rejects_hostile_sizes() {
// Declared total larger than the chunk.
let mut c = 1000u64.to_be_bytes().to_vec();
c.extend_from_slice(&1000u32.to_be_bytes());
c.extend_from_slice(&[0u8; 8]);
assert!(lz4_decompress(&c, 64).is_err());
// Truncated block.
let mut c = 16u64.to_be_bytes().to_vec();
c.extend_from_slice(&16u32.to_be_bytes());
c.extend_from_slice(&16u32.to_be_bytes());
c.extend_from_slice(&[1u8; 4]);
assert!(lz4_decompress(&c, 16).is_err());
}
#[test]
#[cfg(feature = "lz4")]
fn lz4_compress_decompress_roundtrip() {
let data: Vec<u8> = (0..256).map(|i| (i % 256) as u8).collect();
let compressed = lz4_compress(&data).unwrap();
let compressed = lz4_compress(&data, &[]).unwrap();
let decompressed = lz4_decompress(&compressed, data.len()).unwrap();
assert_eq!(decompressed, data);
}
+10
View File
@@ -0,0 +1,10 @@
# Filter conformance fixtures
Files written by libhdf5 (and its registered filter plugins), used by the
filter regression tests in `src/filters.rs` to compare our decoders against
the values h5py/libhdf5 read from the same bytes. Chunk byte ranges quoted in
the tests come from h5py's `DatasetID.get_chunk_info`.
| File | Origin | Licence |
|------|--------|---------|
| `h5ex_d_lz4.h5` | HDF Group `HDF5Examples/C/H5FLT/tfiles/h5ex_d_lz4.h5` (hdf5 repository) | HDF5 licence (BSD-3-Clause style) |
Binary file not shown.
@@ -940,3 +940,48 @@ fn provenance_verify_written_file() {
.unwrap();
assert_eq!(result, clawhdf5_format::provenance::VerifyResult::Ok);
}
// ---- hdf5plugin interop: registered third-party compression filters ----
/// Write `data` (f64, 1-D, chunked) with `configure` applied, then read it
/// back with h5py + hdf5plugin (libhdf5's registered filter plugins) and
/// return the values it decodes.
#[cfg(any(feature = "lz4", feature = "zstd"))]
fn hdf5plugin_roundtrip(
tag: &str,
data: &[f64],
configure: impl FnOnce(&mut clawhdf5_format::type_builders::DatasetBuilder),
) -> Vec<f64> {
let mut fw = FileWriter::new();
let ds = fw.create_dataset("data");
ds.with_f64_data(data)
.with_shape(&[data.len() as u64])
.with_chunks(&[250]);
configure(ds);
let bytes = fw.finish().unwrap();
let path = std::env::temp_dir().join(format!("clawhdf5_hdf5plugin_{tag}.h5"));
std::fs::write(&path, &bytes).unwrap();
let script = format!(
"import h5py,hdf5plugin,json; f=h5py.File('{}','r'); print(json.dumps(f['data'][:].tolist()))",
path.display()
);
let stdout = h5py_read(&path, &script);
serde_json::from_str(&stdout).unwrap()
}
/// libhdf5's LZ4 plugin must decode what we write (it could not while we
/// wrote a private 4-byte-LE-size framing).
#[cfg(feature = "lz4")]
#[test]
#[ignore = "requires Python h5py + hdf5plugin"]
fn hdf5plugin_reads_our_lz4() {
let data: Vec<f64> = (0..1000).map(|i| (i % 37) as f64 * 0.5).collect();
let got = hdf5plugin_roundtrip("lz4", &data, |ds| {
ds.with_lz4();
});
assert_eq!(got, data);
let got = hdf5plugin_roundtrip("lz4_noshuffle", &data, |ds| {
ds.with_lz4().without_shuffle();
});
assert_eq!(got, data);
}