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) <[email protected]>
This commit is contained in:
osobh
2026-09-26 00:30:40 -05:00
co-authored by Claude Opus 5.5
parent e73ac2af09
commit 7d7a7e75d4
7 changed files with 164 additions and 9 deletions
+11
View File
@@ -724,6 +724,17 @@ fn main() {
return; 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)); top.insert("superblock_version".into(), json!(sb.version));
let ctx = Ctx { let ctx = Ctx {
data: hdf5, data: hdf5,
+18
View File
@@ -214,6 +214,14 @@ pub enum FormatError {
/// dimension, a rank that does not match the dataspace, or a chunk of /// dimension, a rank that does not match the dataspace, or a chunk of
/// 4 GiB or more. /// 4 GiB or more.
InvalidChunkDimensions(String), 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 { impl fmt::Display for FormatError {
@@ -467,6 +475,16 @@ impl fmt::Display for FormatError {
FormatError::InvalidChunkDimensions(why) => { FormatError::InvalidChunkDimensions(why) => {
write!(f, "invalid chunk dimensions: {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"
)
}
} }
} }
} }
+60
View File
@@ -100,6 +100,42 @@ pub mod swmr_flags {
} }
impl Superblock { 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<u64, FormatError> {
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. /// Whether the file was opened with write access when the superblock was written.
pub fn is_write_access(&self) -> bool { pub fn is_write_access(&self) -> bool {
self.consistency_flags & swmr_flags::WRITE_ACCESS != 0 self.consistency_flags & swmr_flags::WRITE_ACCESS != 0
@@ -537,6 +573,30 @@ mod tests {
buf 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] #[test]
fn parse_v0_8byte_offsets() { fn parse_v0_8byte_offsets() {
let data = build_v0_bytes(8); let data = build_v0_bytes(8);
+8 -1
View File
@@ -43,6 +43,8 @@ pub struct LazyFile<R: HDF5Read> {
/// Offset of the superblock in the file (the user-block size); every /// Offset of the superblock in the file (the user-block size); every
/// HDF5 address is relative to it. /// HDF5 address is relative to it.
base: usize, base: usize,
/// End of the HDF5 data (`Superblock::data_end`, absolute).
end: usize,
superblock: Superblock, superblock: Superblock,
root_header: ObjectHeader, root_header: ObjectHeader,
/// Cache of parsed object headers, keyed by address. /// Cache of parsed object headers, keyed by address.
@@ -74,9 +76,13 @@ impl<R: HDF5Read> LazyFile<R> {
/// ///
/// Parses only the superblock and root group object header. /// Parses only the superblock and root group object header.
pub fn open(reader: R) -> Result<Self, Error> { pub fn open(reader: R) -> Result<Self, Error> {
let whole_len = reader.as_bytes().len() as u64;
let (user_block, data) = signature::split_user_block(reader.as_bytes())?; let (user_block, data) = signature::split_user_block(reader.as_bytes())?;
let base = user_block.len(); let base = user_block.len();
let superblock = Superblock::parse(data, 0)?; 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( let root_header = ObjectHeader::parse(
data, data,
superblock.root_group_address as usize, superblock.root_group_address as usize,
@@ -86,6 +92,7 @@ impl<R: HDF5Read> LazyFile<R> {
Ok(Self { Ok(Self {
reader, reader,
base, base,
end,
superblock, superblock,
root_header, root_header,
header_cache: RefCell::new(HashMap::new()), header_cache: RefCell::new(HashMap::new()),
@@ -104,7 +111,7 @@ impl<R: HDF5Read> LazyFile<R> {
} }
fn hdf5_bytes(&self) -> &[u8] { 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. /// Returns a reference to the parsed superblock.
+7 -1
View File
@@ -35,6 +35,8 @@ pub struct MmapFile {
/// Offset of the superblock in the mapped file (the user-block size); /// Offset of the superblock in the mapped file (the user-block size);
/// every HDF5 address is relative to it. /// every HDF5 address is relative to it.
base: usize, base: usize,
/// End of the HDF5 data (`Superblock::data_end`, absolute).
end: usize,
superblock: Superblock, superblock: Superblock,
} }
@@ -42,12 +44,16 @@ impl MmapFile {
/// Open an HDF5 file using memory-mapped I/O. /// Open an HDF5 file using memory-mapped I/O.
pub fn open<P: AsRef<std::path::Path>>(path: P) -> Result<Self, Error> { pub fn open<P: AsRef<std::path::Path>>(path: P) -> Result<Self, Error> {
let reader = MmapReader::open(path).map_err(Error::Io)?; 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 (user_block, data) = signature::split_user_block(reader.as_bytes())?;
let base = user_block.len(); let base = user_block.len();
let superblock = Superblock::parse(data, 0)?; 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 { Ok(Self {
reader, reader,
base, base,
end,
superblock, superblock,
}) })
} }
@@ -55,7 +61,7 @@ impl MmapFile {
/// The file's bytes from the superblock on — the space HDF5 addresses /// The file's bytes from the superblock on — the space HDF5 addresses
/// index into. /// index into.
fn hdf5_bytes(&self) -> &[u8] { 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). /// Size of the user block before the superblock (0 for most files).
+15 -7
View File
@@ -45,26 +45,34 @@ impl Backing {
} }
} }
/// The file's bytes, viewed from the superblock on. A file may start with a /// The file's bytes, viewed from the superblock on and up to the end of
/// user block (the superblock at 512, 1024, …); every HDF5 address is /// file the superblock records. A file may start with a user block (the
/// relative to the superblock, so all parsing goes through [`Self::as_bytes`]. /// superblock at 512, 1024, …); every HDF5 address is relative to the
/// superblock, so all parsing goes through [`Self::as_bytes`].
struct FileData { struct FileData {
backing: Backing, backing: Backing,
/// Offset of the superblock in the file (the user-block size). /// Offset of the superblock in the file (the user-block size).
base: usize, base: usize,
/// End of the HDF5 data in the file (`Superblock::data_end`, absolute).
end: usize,
} }
impl FileData { 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> { 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 base = user_block.len();
let superblock = Superblock::parse(hdf5, 0)?; 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] { fn as_bytes(&self) -> &[u8] {
&self.backing.whole_file()[self.base..] &self.backing.whole_file()[self.base..self.end]
} }
fn len(&self) -> usize { fn len(&self) -> usize {
@@ -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="<i4"))
f.attrs["note"] = "x" * 64
data = open(good, "rb").read()
open(os.path.join(d, f"{libver}_truncated.h5"), "wb").write(data[:-8])
open(os.path.join(d, f"{libver}_appended.h5"), "wb").write(data + b"\0" * 64)
open(os.path.join(d, f"{libver}_moved.h5"), "wb").write(b"\0" * 512 + data)
"#,
);
assert_agrees_with_h5py(
dir.path(),
&verdicts,
&[
"earliest_appended ok",
"earliest_good ok",
"earliest_moved ok",
"earliest_truncated ERROR",
"latest_appended ok",
"latest_good ok",
"latest_moved ok",
"latest_truncated ERROR",
],
);
for name in ["earliest_truncated", "latest_truncated"] {
let err = File::open(dir.path().join(format!("{name}.h5")))
.err()
.unwrap_or_else(|| panic!("{name} opened"));
assert!(err.to_string().contains("truncated file"), "{name}: {err}");
}
}