goblin: Guard every with_capacity call with a bounds check (#298)

* Guard every with_capacity call with a bounds check
The crate does a lot of pre-allocation based on untrusted input, which
can lead to unbounded allocation.
* Introduce a new Error Variant for Buffer bounds checks

src/archive/mod.rs

@@ -216,6 +216,11 @@ impl<'a> Index<'a> {
     pub fn parse_sysv_index(buffer: &'a [u8]) -> Result<Self> {
         let offset = &mut 0;
         let sizeof_table = buffer.gread_with::<u32>(offset, scroll::BE)? as usize;
+
+        if sizeof_table > buffer.len() / 4 {
+            return Err(Error::BufferTooShort(sizeof_table, "indices"));
+        }
+
         let mut indexes = Vec::with_capacity(sizeof_table);
         for _ in 0..sizeof_table {
             indexes.push(buffer.gread_with::<u32>(offset, scroll::BE)?);
@@ -270,6 +275,10 @@ impl<'a> Index<'a> {
         let strtab_bytes = buffer.pread_with::<u32>(entries_bytes + 4, scroll::LE)? as usize;
         let strtab = strtab::Strtab::parse(buffer, entries_bytes + 8, strtab_bytes, 0x0)?;
 
+        if entries_bytes > buffer.len() {
+            return Err(Error::BufferTooShort(entries, "entries"));
+        }
+
         // build the index
         let mut indexes = Vec::with_capacity(entries);
         let mut strings = Vec::with_capacity(entries);
@@ -311,11 +320,22 @@ impl<'a> Index<'a> {
     pub fn parse_windows_linker_member(buffer: &'a [u8]) -> Result<Self> {
         let offset = &mut 0;
         let members = buffer.gread_with::<u32>(offset, scroll::LE)? as usize;
+
+        if members > buffer.len() / 4 {
+            return Err(Error::BufferTooShort(members, "members"));
+        }
+
         let mut member_offsets = Vec::with_capacity(members);
         for _ in 0..members {
             member_offsets.push(buffer.gread_with::<u32>(offset, scroll::LE)?);
         }
+
         let symbols = buffer.gread_with::<u32>(offset, scroll::LE)? as usize;
+
+        if symbols > buffer.len() / 2 {
+            return Err(Error::BufferTooShort(symbols, "symbols"));
+        }
+
         let mut symbol_offsets = Vec::with_capacity(symbols);
         for _ in 0..symbols {
             symbol_offsets

src/elf/dynamic.rs

@@ -427,6 +427,7 @@ if_alloc! {
                         &[]
                     };
                     let size = Dyn::size_with(&ctx);
+                    // the validity of `count` was implicitly checked by reading `bytes`.
                     let count = filesz / size;
                     let mut dyns = Vec::with_capacity(count);
                     let mut offset = 0;
@@ -449,7 +450,7 @@ if_alloc! {
 
         pub fn get_libraries<'a>(&self, strtab: &Strtab<'a>) -> Vec<&'a str> {
             use log::warn;
-            let count = self.info.needed_count;
+            let count = self.info.needed_count.min(self.dyns.len());
             let mut needed = Vec::with_capacity(count);
             for dynamic in &self.dyns {
                 if dynamic.d_tag as u64 == DT_NEEDED {
@@ -565,7 +566,7 @@ macro_rules! elf_dyn_std_impl {
 
             /// Gets the needed libraries from the `_DYNAMIC` array, with the str slices lifetime tied to the dynamic array/strtab's lifetime(s)
             pub unsafe fn get_needed<'a>(dyns: &[Dyn], strtab: *const Strtab<'a>, count: usize) -> Vec<&'a str> {
-                let mut needed = Vec::with_capacity(count);
+                let mut needed = Vec::with_capacity(count.min(dyns.len()));
                 for dynamic in dyns {
                     if u64::from(dynamic.d_tag) == DT_NEEDED {
                         let lib = &(*strtab)[dynamic.d_val as usize];

src/elf/program_header.rs

@@ -161,8 +161,7 @@ if_alloc! {
             use scroll::Pread;
             // Sanity check to avoid OOM
             if count > bytes.len() / Self::size(ctx) {
-                let message = format!("Buffer is too short for {} program headers", count);
-                return Err(error::Error::Malformed(message));
+                return Err(error::Error::BufferTooShort(count, "program headers"));
             }
             let mut program_headers = Vec::with_capacity(count);
             for _ in 0..count {

src/elf/section_header.rs

@@ -459,8 +459,7 @@ if_alloc! {
 
             // Sanity check to avoid OOM
             if count > bytes.len() / Self::size(ctx) {
-                let message = format!("Buffer is too short for {} section headers", count);
-                return Err(error::Error::Malformed(message));
+                return Err(error::Error::BufferTooShort(count, "section headers"));
             }
             let mut section_headers = Vec::with_capacity(count);
             section_headers.push(empty_sh);

src/elf/sym.rs

@@ -388,6 +388,9 @@ impl Sym {
     #[cfg(feature = "endian_fd")]
     /// Parse `count` vector of ELF symbols from `offset`
     pub fn parse(bytes: &[u8], mut offset: usize, count: usize, ctx: Ctx) -> Result<Vec<Sym>> {
+        if count > bytes.len() / Sym::size_with(&ctx) {
+            return Err(crate::error::Error::BufferTooShort(count, "symbols"));
+        }
         let mut syms = Vec::with_capacity(count);
         for _ in 0..count {
             let sym = bytes.gread_with(&mut offset, ctx)?;

src/error.rs

@@ -7,6 +7,7 @@ use core::result;
 #[cfg(feature = "std")]
 use std::{error, io};
 
+#[non_exhaustive]
 #[derive(Debug)]
 /// A custom Goblin error
 pub enum Error {
@@ -19,6 +20,8 @@ pub enum Error {
     /// An IO based error
     #[cfg(feature = "std")]
     IO(io::Error),
+    /// Buffer is too short to hold N items
+    BufferTooShort(usize, &'static str),
 }
 
 #[cfg(feature = "std")]
@@ -27,8 +30,7 @@ impl error::Error for Error {
         match *self {
             Error::IO(ref io) => Some(io),
             Error::Scroll(ref scroll) => Some(scroll),
-            Error::BadMagic(_) => None,
-            Error::Malformed(_) => None,
+            _ => None,
         }
     }
 }
@@ -54,6 +56,7 @@ impl fmt::Display for Error {
             Error::Scroll(ref err) => write!(fmt, "{}", err),
             Error::BadMagic(magic) => write!(fmt, "Invalid magic number: 0x{:x}", magic),
             Error::Malformed(ref msg) => write!(fmt, "Malformed entity: {}", msg),
+            Error::BufferTooShort(n, item) => write!(fmt, "Buffer is too short for {} {}", n, item),
         }
     }
 }

src/mach/exports.rs

@@ -199,6 +199,9 @@ impl<'a> ExportTrie<'a> {
         current_symbol: String,
         mut offset: usize,
     ) -> error::Result<Vec<(String, usize)>> {
+        if nbranches > self.data.len() {
+            return Err(error::Error::BufferTooShort(nbranches, "branches"));
+        }
         let mut branches = Vec::with_capacity(nbranches);
         //println!("\t@{:#x}", *offset);
         for _i in 0..nbranches {

src/mach/mod.rs

@@ -162,6 +162,13 @@ impl<'a> MachO<'a> {
         let is_64 = ctx.container.is_big();
         *offset += header::Header::size_with(&ctx.container);
         let ncmds = header.ncmds;
+
+        let sizeofcmds = header.sizeofcmds as usize;
+        // a load cmd is at least 2 * 4 bytes, (type, sizeof)
+        if ncmds > sizeofcmds / 8 || sizeofcmds > bytes.len() {
+            return Err(error::Error::BufferTooShort(ncmds, "load commands"));
+        }
+
         let mut cmds: Vec<load_command::LoadCommand> = Vec::with_capacity(ncmds);
         let mut symbols = None;
         let mut libs = vec!["self"];
@@ -364,6 +371,10 @@ impl<'a> MultiArch<'a> {
     }
     /// Return all the architectures in this binary
     pub fn arches(&self) -> error::Result<Vec<fat::FatArch>> {
+        if self.narches > self.data.len() / fat::SIZEOF_FAT_ARCH {
+            return Err(error::Error::BufferTooShort(self.narches, "arches"));
+        }
+
         let mut arches = Vec::with_capacity(self.narches);
         for arch in self.iter_arches() {
             arches.push(arch?);

src/pe/export.rs

@@ -108,6 +108,19 @@ impl<'a> ExportData<'a> {
         let number_of_name_pointers = export_directory_table.number_of_name_pointers as usize;
         let address_table_entries = export_directory_table.address_table_entries as usize;
 
+        if number_of_name_pointers > bytes.len() {
+            return Err(error::Error::BufferTooShort(
+                number_of_name_pointers,
+                "name pointers",
+            ));
+        }
+        if address_table_entries > bytes.len() {
+            return Err(error::Error::BufferTooShort(
+                address_table_entries,
+                "address table entries",
+            ));
+        }
+
         let export_name_pointer_table = utils::find_offset(
             export_directory_table.name_pointer_rva as usize,
             sections,

src/pe/header.rs

@@ -3,6 +3,7 @@ use crate::pe::{optional_header, section_table, symbol};
 use crate::strtab;
 use alloc::vec::Vec;
 use log::debug;
+use scroll::ctx::SizeWith;
 use scroll::{IOread, IOwrite, Pread, Pwrite, SizeWith};
 
 /// DOS header present in all PE binaries
@@ -143,6 +144,12 @@ impl CoffHeader {
         offset: &mut usize,
     ) -> error::Result<Vec<section_table::SectionTable>> {
         let nsections = self.number_of_sections as usize;
+
+        // a section table is at least 40 bytes
+        if nsections > bytes.len() / 40 {
+            return Err(error::Error::BufferTooShort(nsections, "sections"));
+        }
+
         let mut sections = Vec::with_capacity(nsections);
         // Note that if we are handling a BigCoff, the size of the symbol will be different!
         let string_table_offset = self.pointer_to_symbol_table as usize

tests/macho.rs

@@ -577,3 +577,10 @@ fn relocations() {
     assert_eq!(reloc.is_pic(), true);
     assert_eq!(reloc.is_extern(), true);
 }
+
+// See https://github.com/getsentry/symbolic/issues/479
+#[test]
+fn fuzzed_memory_growth() {
+    let bytes = b"\xfe\xed\xfa\xce\xce\xfa\xff\xfe\xcf*\x06;\xfe\xfa\xce\xff\xff\xff\xff0\xce:\xfa\xffj\xfe\xcf*\x06\x00;\xc6";
+    assert!(Mach::parse(&bytes[..]).is_err());
+}