From 7d7a7e75d406638b177d6cc4d57d82ae4f26609f Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 00:30:35 -0500 Subject: [PATCH] fix: refuse truncated files and read nothing past the recorded end of file The superblock records where the file's data ends. libhdf5 refuses to open a file shorter than that ("truncated file", H5F__super_read) and fails any read past it ("addr overflow" / "address plus size exceeds file eoa"). clawhdf5 read whatever was left of a truncated file, and read bytes after the recorded end as if they belonged to the file. New Superblock::data_end: FormatError::TruncatedFile for a file shorter than its recorded end, otherwise where the HDF5 data ends. As in libhdf5, the recorded end moves with the superblock when its recorded base address is not where it is (a user block added afterwards; cve-2021-36977, which h5py reads, depends on it), and the check is skipped for a v3 superblock still being written in SWMR mode. File, LazyFile, MmapFile and the conformance probe refuse a truncated file and parse only up to the end. Files clawhdf5 writes record their true length, and the v2.5.0 agent-store fixture and files written by v2.7.0 (plain, paged, user-block free) pass the check. Interop test: h5py writes a file; a copy missing its last 8 bytes must be refused by both, a copy with bytes appended and one moved behind a new 512-byte user block must read in both. Conformance (cached corpus, tank): 570 -> 571 ok (h5clear_fsm_persist_less.h5, whose data past the recorded end was being read); ten files h5py refuses as truncated (cve-2018-13874, cve-2018-13876, the family/multi/subfiling members, h5clear_fsm_persist_ greater/user_greater) are now refused at open instead of read. Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/probe/src/main.rs | 11 ++++ crates/clawhdf5-format/src/error.rs | 18 ++++++ crates/clawhdf5-format/src/superblock.rs | 60 +++++++++++++++++++ crates/clawhdf5/src/lazy.rs | 9 ++- crates/clawhdf5/src/mmap_file.rs | 8 ++- crates/clawhdf5/src/reader.rs | 22 ++++--- .../tests/header_validation_interop.rs | 45 ++++++++++++++ 7 files changed, 164 insertions(+), 9 deletions(-) diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 9629dcc..87c660d 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -724,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, diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index 814a5a7..b3cfd65 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -214,6 +214,14 @@ pub enum FormatError { /// dimension, a rank that does not match the dataspace, or a chunk of /// 4 GiB or more. 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 { @@ -467,6 +475,16 @@ impl fmt::Display for FormatError { 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/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/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index cf70f70..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. diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 8c35163..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). diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 5f2a027..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 { diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs index a2f3e7a..23509e1 100644 --- a/crates/clawhdf5/tests/header_validation_interop.rs +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -153,3 +153,48 @@ for libver in ("earliest", "latest"): ], ); } + +#[test] +fn truncated_files_are_refused_and_nothing_past_the_end_of_file_is_read() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + // The superblock records where the file's data ends. A copy missing its + // last bytes is truncated: libhdf5 refuses to open it (this read what + // was left). Bytes appended after the end are not part of the file, and + // a superblock moved by prepending a user block (its recorded base + // address now wrong) has its end of file moved with it; both still read. + 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="