fix(format): bound what a VL read retains on a crafted global heap
VlResolver kept an owned copy of every object of every heap collection it parsed, for the whole read. Collections nested inside each other's object data, 32 bytes apart with each element pointing at a different one, made retained memory O(elements x file size): 1.58 GB for a 744 KB file (read_vl_strings did the same before VlResolver). Chaining every collection's objects into one shared run of tiny objects made parse time O(elements x objects) as well. libhdf5 refuses these files. - The cache records where each object lies (GlobalHeapCollection:: parse_index, new) instead of copying it, and is dropped past a 32 MiB budget. - A collection overlapping one already read is an error: libhdf5 gives every collection its own block, so only a crafted file has them. - parse and parse_index refuse a collection that runs past the end of the file and an object that runs past the end of its collection. tests/vl_heap_bounds.rs measures peak heap use with a counting allocator: 129 MB and 350 MB live before on its two crafted files (64 KB and 176 KB), 97 KB and 0.9 MB now. Conformance unchanged at 575 of 697. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
@@ -10,7 +10,7 @@ use alloc::{collections::BTreeMap, format, string::String, vec, vec::Vec};
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use crate::error::FormatError;
|
||||
use crate::global_heap::GlobalHeapCollection;
|
||||
use crate::global_heap::{GlobalHeapCollection, GlobalHeapIndex};
|
||||
|
||||
/// A parsed variable-length element reference (global heap ID).
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -134,38 +134,43 @@ pub fn check_element_size(stored_size: u32, offset_size: u8) -> Result<(), Forma
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A parsed collection, with its objects indexed for lookup.
|
||||
/// A collection's objects, located in the file data but not copied:
|
||||
/// `(index, offset, size)` of the first object with each index, sorted by
|
||||
/// index.
|
||||
struct CachedCollection {
|
||||
collection: GlobalHeapCollection,
|
||||
/// `slots[index]` is the position in `collection.objects` of the first
|
||||
/// object with that index.
|
||||
slots: Vec<Option<usize>>,
|
||||
objects: Vec<(u16, usize, usize)>,
|
||||
}
|
||||
|
||||
impl CachedCollection {
|
||||
fn new(collection: GlobalHeapCollection) -> Self {
|
||||
let max = collection
|
||||
fn new(index: GlobalHeapIndex) -> Self {
|
||||
let mut objects: Vec<(u16, usize, usize)> = index
|
||||
.objects
|
||||
.iter()
|
||||
.map(|o| o.index as usize)
|
||||
.max()
|
||||
.unwrap_or(0);
|
||||
let mut slots = vec![None; max + 1];
|
||||
for (pos, obj) in collection.objects.iter().enumerate() {
|
||||
let slot = &mut slots[obj.index as usize];
|
||||
if slot.is_none() {
|
||||
*slot = Some(pos);
|
||||
}
|
||||
}
|
||||
Self { collection, slots }
|
||||
.map(|o| (o.index, o.offset, o.size))
|
||||
.collect();
|
||||
// Stable, so the first object with a repeated index is kept.
|
||||
objects.sort_by_key(|o| o.0);
|
||||
objects.dedup_by_key(|o| o.0);
|
||||
Self { objects }
|
||||
}
|
||||
|
||||
fn get(&self, index: u32) -> Option<&[u8]> {
|
||||
let pos = (*self.slots.get(usize::try_from(index).ok()?)?)?;
|
||||
Some(&self.collection.objects[pos].data)
|
||||
/// What this entry costs to keep, in bytes (roughly).
|
||||
fn cost(&self) -> usize {
|
||||
64 + self.objects.len() * core::mem::size_of::<(u16, usize, usize)>()
|
||||
}
|
||||
|
||||
fn get(&self, index: u32) -> Option<(usize, usize)> {
|
||||
let index = u16::try_from(index).ok()?;
|
||||
let i = self.objects.binary_search_by_key(&index, |o| o.0).ok()?;
|
||||
Some((self.objects[i].1, self.objects[i].2))
|
||||
}
|
||||
}
|
||||
|
||||
/// How many bytes of collection indexes a [`VlResolver`] keeps before it
|
||||
/// drops them and starts again. Values are never copied into the cache, so
|
||||
/// this bounds what a read retains however many collections it visits.
|
||||
const CACHE_BUDGET: usize = 32 << 20;
|
||||
|
||||
/// Resolves variable-length elements against a file's global heap, parsing
|
||||
/// each heap collection once however many elements point into it.
|
||||
///
|
||||
@@ -173,11 +178,22 @@ impl CachedCollection {
|
||||
/// empty string or sequence), and an element whose heap object is not
|
||||
/// exactly `length × base size` bytes is an error ("Expected global heap
|
||||
/// object size does not match"), not a truncated or padded value.
|
||||
///
|
||||
/// Memory stays bounded on hostile files: the cache holds where each
|
||||
/// object lies, not a copy of it, up to a fixed budget; and collections
|
||||
/// that overlap one another are refused (libhdf5 never writes them), so a
|
||||
/// file cannot make the resolver parse the same bytes as the objects of
|
||||
/// many collections.
|
||||
pub struct VlResolver<'a> {
|
||||
file_data: &'a [u8],
|
||||
offset_size: u8,
|
||||
length_size: u8,
|
||||
cache: BTreeMap<u64, CachedCollection>,
|
||||
cached_bytes: usize,
|
||||
budget: usize,
|
||||
/// Start → end of every collection parsed so far (kept when the cache
|
||||
/// is dropped, to check overlaps).
|
||||
extents: BTreeMap<usize, usize>,
|
||||
}
|
||||
|
||||
impl<'a> VlResolver<'a> {
|
||||
@@ -189,6 +205,9 @@ impl<'a> VlResolver<'a> {
|
||||
offset_size,
|
||||
length_size,
|
||||
cache: BTreeMap::new(),
|
||||
cached_bytes: 0,
|
||||
budget: CACHE_BUDGET,
|
||||
extents: BTreeMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -210,11 +229,18 @@ impl<'a> VlResolver<'a> {
|
||||
}
|
||||
|
||||
/// The bytes of one element: `length × base_size` bytes from the heap,
|
||||
/// or empty for a null or zero-length element.
|
||||
fn resolve(&mut self, vl: &VlElement, base_size: usize) -> Result<&[u8], FormatError> {
|
||||
/// or `None` for a null element.
|
||||
fn resolve(
|
||||
&mut self,
|
||||
vl: &VlElement,
|
||||
base_size: usize,
|
||||
) -> Result<Option<&'a [u8]>, FormatError> {
|
||||
let addr = vl.collection_address;
|
||||
if addr == 0 || (vl.length == 0 && is_undefined_address(addr, self.offset_size)) {
|
||||
return Ok(&[]);
|
||||
if addr == 0 {
|
||||
return Ok(None);
|
||||
}
|
||||
if vl.length == 0 && is_undefined_address(addr, self.offset_size) {
|
||||
return Ok(Some(&[]));
|
||||
}
|
||||
let data = self.object(vl)?;
|
||||
let expected = (vl.length as usize)
|
||||
@@ -229,7 +255,27 @@ impl<'a> VlResolver<'a> {
|
||||
vl.length
|
||||
)));
|
||||
}
|
||||
Ok(data)
|
||||
Ok(Some(data))
|
||||
}
|
||||
|
||||
/// One element (the first [`element_size`](Self::element_size) bytes of
|
||||
/// `elem`) of a variable-length sequence whose base type is `base_size`
|
||||
/// bytes: its `length × base_size` bytes, or `None` for a null element
|
||||
/// (heap address 0).
|
||||
pub fn element(
|
||||
&mut self,
|
||||
elem: &[u8],
|
||||
base_size: usize,
|
||||
) -> Result<Option<&'a [u8]>, FormatError> {
|
||||
let vl = parse_vl_references(elem, 1, self.offset_size)?;
|
||||
self.resolve(&vl[0], base_size)
|
||||
}
|
||||
|
||||
/// One variable-length string element: its bytes up to the first NUL,
|
||||
/// or `None` for a null element (h5dump prints it as `NULL`, h5py
|
||||
/// returns it as empty).
|
||||
pub fn string_element(&mut self, elem: &[u8]) -> Result<Option<&'a [u8]>, FormatError> {
|
||||
Ok(self.element(elem, 1)?.map(cut_at_nul))
|
||||
}
|
||||
|
||||
/// The strings of the variable-length string elements in `raw`, as
|
||||
@@ -238,11 +284,7 @@ impl<'a> VlResolver<'a> {
|
||||
pub fn string_bytes(&mut self, raw: &[u8]) -> Result<Vec<Vec<u8>>, FormatError> {
|
||||
self.elements(raw)?
|
||||
.iter()
|
||||
.map(|vl| {
|
||||
let s = self.resolve(vl, 1)?;
|
||||
let end = s.iter().position(|&b| b == 0).unwrap_or(s.len());
|
||||
Ok(s[..end].to_vec())
|
||||
})
|
||||
.map(|vl| Ok(self.resolve(vl, 1)?.map(cut_at_nul).unwrap_or(&[]).to_vec()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
@@ -270,11 +312,16 @@ impl<'a> VlResolver<'a> {
|
||||
}
|
||||
self.elements(raw)?
|
||||
.iter()
|
||||
.map(|vl| self.resolve(vl, base_size).map(<[u8]>::to_vec))
|
||||
.map(|vl| Ok(self.resolve(vl, base_size)?.unwrap_or(&[]).to_vec()))
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
/// A string's bytes up to its first NUL.
|
||||
fn cut_at_nul(s: &[u8]) -> &[u8] {
|
||||
&s[..s.iter().position(|&b| b == 0).unwrap_or(s.len())]
|
||||
}
|
||||
|
||||
/// Resolve VL strings from raw data by looking up each element in the global heap.
|
||||
///
|
||||
/// Reads the first `num_elements` elements of `raw`. Strings end at their
|
||||
@@ -342,25 +389,66 @@ pub fn read_vl_bytes(
|
||||
Ok(result)
|
||||
}
|
||||
|
||||
impl VlResolver<'_> {
|
||||
impl<'a> VlResolver<'a> {
|
||||
/// The heap object `vl` points to, whatever its size; its collection is
|
||||
/// parsed on first use.
|
||||
fn object(&mut self, vl: &VlElement) -> Result<&[u8], FormatError> {
|
||||
fn object(&mut self, vl: &VlElement) -> Result<&'a [u8], FormatError> {
|
||||
let addr = vl.collection_address;
|
||||
if !self.cache.contains_key(&addr) {
|
||||
let offset = usize::try_from(addr).map_err(|_| FormatError::UnexpectedEof {
|
||||
expected: usize::MAX,
|
||||
available: self.file_data.len(),
|
||||
})?;
|
||||
let coll = GlobalHeapCollection::parse(self.file_data, offset, self.length_size)?;
|
||||
self.cache.insert(addr, CachedCollection::new(coll));
|
||||
let index =
|
||||
GlobalHeapCollection::parse_index(self.file_data, offset, self.length_size)?;
|
||||
// parse_index checked that the collection lies in the file.
|
||||
let end = offset + index.collection_size as usize;
|
||||
self.check_overlap(offset, end)?;
|
||||
let coll = CachedCollection::new(index);
|
||||
if self.cached_bytes.saturating_add(coll.cost()) > self.budget {
|
||||
self.cache.clear();
|
||||
self.cached_bytes = 0;
|
||||
}
|
||||
self.cached_bytes += coll.cost();
|
||||
self.cache.insert(addr, coll);
|
||||
}
|
||||
self.cache[&addr]
|
||||
.get(vl.object_index)
|
||||
.ok_or(FormatError::GlobalHeapObjectNotFound {
|
||||
let (start, size) = self.cache[&addr].get(vl.object_index).ok_or(
|
||||
FormatError::GlobalHeapObjectNotFound {
|
||||
collection_address: addr,
|
||||
index: vl.object_index as u16,
|
||||
})
|
||||
},
|
||||
)?;
|
||||
Ok(&self.file_data[start..start + size])
|
||||
}
|
||||
|
||||
/// Record the collection at `start..end`, refusing one that overlaps a
|
||||
/// collection already read. libhdf5 allocates each collection its own
|
||||
/// block; overlapping ones only come from a crafted file, where they let
|
||||
/// every byte be parsed again as the objects of each collection.
|
||||
fn check_overlap(&mut self, start: usize, end: usize) -> Result<(), FormatError> {
|
||||
if let Some(&known) = self.extents.get(&start) {
|
||||
return if known == end {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(FormatError::VlDataError(format!(
|
||||
"global heap collection at {start} changed size"
|
||||
)))
|
||||
};
|
||||
}
|
||||
let before = self.extents.range(..start).next_back();
|
||||
let after = self.extents.range(start..).next();
|
||||
let clash = match (before, after) {
|
||||
(Some((&s, &e)), _) if e > start => Some(s),
|
||||
(_, Some((&s, _))) if s < end => Some(s),
|
||||
_ => None,
|
||||
};
|
||||
if let Some(other) = clash {
|
||||
return Err(FormatError::VlDataError(format!(
|
||||
"global heap collection at {start} overlaps the one at {other}"
|
||||
)));
|
||||
}
|
||||
self.extents.insert(start, end);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -581,6 +669,43 @@ mod tests {
|
||||
assert!(r.strings(&raw[..30]).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_cache_stays_within_its_budget_and_rereads_what_it_dropped() {
|
||||
// Twenty collections of three objects each; a budget that holds
|
||||
// about two of them. Reading every element twice must still return
|
||||
// the right strings after the cache is dropped.
|
||||
let mut file_data = vec![0u8; 64];
|
||||
let mut raw = Vec::new();
|
||||
for c in 0..20u64 {
|
||||
let at = file_data.len();
|
||||
let names: Vec<String> = (0..3).map(|i| format!("c{c}o{i}")).collect();
|
||||
let objs: Vec<(u16, &[u8])> = names
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, n)| (i as u16 + 1, n.as_bytes()))
|
||||
.collect();
|
||||
build_gcol_at(&mut file_data, at, &objs);
|
||||
for (i, n) in names.iter().enumerate() {
|
||||
raw.extend(element(n.len() as u32, at as u64, i as u32 + 1, 8));
|
||||
}
|
||||
}
|
||||
raw.extend(raw.clone());
|
||||
let mut r = VlResolver::new(&file_data, 8, 8);
|
||||
let one = CachedCollection {
|
||||
objects: vec![(0, 0, 0); 3],
|
||||
}
|
||||
.cost();
|
||||
r.budget = 2 * one + 1;
|
||||
let want: Vec<String> = (0..2)
|
||||
.flat_map(|_| (0..20).flat_map(|c| (0..3).map(move |i| format!("c{c}o{i}"))))
|
||||
.collect();
|
||||
for (k, chunk) in raw.chunks(16).enumerate() {
|
||||
assert_eq!(r.strings(chunk).unwrap(), [want[k].clone()]);
|
||||
assert!(r.cached_bytes <= r.budget);
|
||||
assert!(r.cache.len() <= 2);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn element_size_is_checked_against_the_offset_size() {
|
||||
assert!(check_element_size(16, 8).is_ok());
|
||||
|
||||
Reference in New Issue
Block a user