Skip to content

Commit

Permalink
read: check that sizes are smaller than the file length in ReadCache (#…
Browse files Browse the repository at this point in the history
…630)

This avoids large allocations due to invalid sizes in corrupted files.
  • Loading branch information
symphorien committed Feb 11, 2024
1 parent e676d8b commit f1a0ec9
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 3 deletions.
30 changes: 28 additions & 2 deletions src/read/read_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,30 @@ struct ReadCacheInternal<R: Read + Seek> {
read: R,
bufs: HashMap<(u64, u64), Box<[u8]>>,
strings: HashMap<(u64, u8), Box<[u8]>>,
len: Option<u64>,
}

impl<R: Read + Seek> ReadCacheInternal<R> {
/// Ensures this range is contained in the len of the file
fn range_in_bounds(&mut self, range: &Range<u64>) -> Result<(), ()> {
if range.start <= range.end && range.end <= self.len()? {
Ok(())
} else {
Err(())
}
}

/// The length of the underlying read, memoized
fn len(&mut self) -> Result<u64, ()> {
match self.len {
Some(len) => Ok(len),
None => {
let len = self.read.seek(SeekFrom::End(0)).map_err(|_| ())?;
self.len = Some(len);
Ok(len)
}
}
}
}

impl<R: Read + Seek> ReadCache<R> {
Expand All @@ -37,6 +61,7 @@ impl<R: Read + Seek> ReadCache<R> {
read,
bufs: HashMap::new(),
strings: HashMap::new(),
len: None,
}),
}
}
Expand Down Expand Up @@ -64,15 +89,15 @@ impl<R: Read + Seek> ReadCache<R> {

impl<'a, R: Read + Seek> ReadRef<'a> for &'a ReadCache<R> {
fn len(self) -> Result<u64, ()> {
let cache = &mut *self.cache.borrow_mut();
cache.read.seek(SeekFrom::End(0)).map_err(|_| ())
self.cache.borrow_mut().len()
}

fn read_bytes_at(self, offset: u64, size: u64) -> Result<&'a [u8], ()> {
if size == 0 {
return Ok(&[]);
}
let cache = &mut *self.cache.borrow_mut();
cache.range_in_bounds(&(offset..(offset.saturating_add(size))))?;
let buf = match cache.bufs.entry((offset, size)) {
Entry::Occupied(entry) => entry.into_mut(),
Entry::Vacant(entry) => {
Expand All @@ -90,6 +115,7 @@ impl<'a, R: Read + Seek> ReadRef<'a> for &'a ReadCache<R> {

fn read_bytes_at_until(self, range: Range<u64>, delimiter: u8) -> Result<&'a [u8], ()> {
let cache = &mut *self.cache.borrow_mut();
cache.range_in_bounds(&range)?;
let buf = match cache.strings.entry((range.start, delimiter)) {
Entry::Occupied(entry) => entry.into_mut(),
Entry::Vacant(entry) => {
Expand Down
47 changes: 47 additions & 0 deletions tests/read/elf.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use object::Object;
use std::path::Path;
use std::path::PathBuf;

#[cfg(feature = "std")]
fn get_buildid(path: &Path) -> Result<Option<Vec<u8>>, object::read::Error> {
let file = std::fs::File::open(path).unwrap();
let reader = object::read::ReadCache::new(file);
let object = object::read::File::parse(&reader)?;
object
.build_id()
.map(|option| option.map(ToOwned::to_owned))
}

#[cfg(feature = "std")]
#[test]
/// Regression test: used to attempt to allocate 5644418395173552131 bytes
fn get_buildid_bad_elf() {
let path: PathBuf = [
"testfiles",
"elf",
"yara-fuzzing",
"crash-7dc27920ae1cb85333e7f2735a45014488134673",
]
.iter()
.collect();
let _ = get_buildid(&path);
}

#[cfg(feature = "std")]
#[test]
fn get_buildid_less_bad_elf() {
let path: PathBuf = [
"testfiles",
"elf",
"yara-fuzzing",
"crash-f1fd008da535b110853885221ebfaac3f262a1c1e280f10929f7b353c44996c8",
]
.iter()
.collect();
let buildid = get_buildid(&path).unwrap().unwrap();
// ground truth obtained from GNU binutils's readelf
assert_eq!(
buildid,
b"\xf9\xc0\xc6\x05\xd3\x76\xbb\xa5\x7e\x02\xf5\x74\x50\x9d\x16\xcc\xe9\x9c\x1b\xf1"
);
}
1 change: 1 addition & 0 deletions tests/read/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "read")]

mod coff;
mod elf;

0 comments on commit f1a0ec9

Please sign in to comment.