From 807786e7eb0b6c95ec9a59f92acc310ee83d24f3 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Mon, 2 Oct 2023 22:21:28 +0200 Subject: [PATCH] pe(coff): COFF symbols and COFF strings are deprecated Previously, I (we?) thought that COFF symbols/strings were always there. In fact, this is not the case and they are deprecated according to https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#coff-file-header-object-and-image > The file offset of the COFF symbol table, or zero > if no COFF symbol table is present. This value should be zero for an image > because COFF debugging information is deprecated. --- src/pe/header.rs | 18 +++++++-- src/pe/mod.rs | 99 ++++++++++++++---------------------------------- src/strtab.rs | 2 +- 3 files changed, 44 insertions(+), 75 deletions(-) diff --git a/src/pe/header.rs b/src/pe/header.rs index 8068cbbbf..06e23c0b0 100644 --- a/src/pe/header.rs +++ b/src/pe/header.rs @@ -262,14 +262,24 @@ impl CoffHeader { } /// Return the COFF symbol table. - pub fn symbols<'a>(&self, bytes: &'a [u8]) -> error::Result> { + pub fn symbols<'a>(&self, bytes: &'a [u8]) -> error::Result>> { let offset = self.pointer_to_symbol_table as usize; let number = self.number_of_symbol_table as usize; - symbol::SymbolTable::parse(bytes, offset, number) + if offset == 0 { + Ok(None) + } else { + symbol::SymbolTable::parse(bytes, offset, number).map(Some) + } } /// Return the COFF string table. - pub fn strings<'a>(&self, bytes: &'a [u8]) -> error::Result> { + pub fn strings<'a>(&self, bytes: &'a [u8]) -> error::Result>> { + // > The file offset of the COFF symbol table, or zero if no COFF symbol table is present. + // > This value should be zero for an image because COFF debugging information is deprecated. + if self.pointer_to_symbol_table == 0 { + return Ok(None); + } + let mut offset = self.pointer_to_symbol_table as usize + symbol::SymbolTable::size(self.number_of_symbol_table as usize); @@ -279,7 +289,7 @@ impl CoffHeader { // The offset needs to be advanced in order to read the strings. offset += length_field_size; - Ok(strtab::Strtab::parse(bytes, offset, length, 0)?) + Ok(Some(strtab::Strtab::parse(bytes, offset, length, 0)?)) } } diff --git a/src/pe/mod.rs b/src/pe/mod.rs index 9421e775e..7ad684100 100644 --- a/src/pe/mod.rs +++ b/src/pe/mod.rs @@ -358,70 +358,26 @@ impl<'a> PE<'a> { written_data_size); } } - // COFF Symbol Table - // Auxiliary Symbol Records - // COFF String Table - { - // Jump offset to COFF Symbol Table, so we can jump to String Table. - let coff_sym_offset = usize::try_from(self.header.coff_header.pointer_to_symbol_table) - .map_err(|_| { - Self::Error::Malformed(format!( - "COFF Header's symbol table offset does not fit in platform `usize`" - )) - })?; - offset = coff_sym_offset; - bytes.gwrite_with( - self.header.coff_header.symbols(self.bytes)?, - &mut offset, - ctx, - )?; - max_offset = max(offset, max_offset); - // TODO: empty string table?! - // let coff_strings = self.header.coff_header.strings(self.bytes)?; - - // FIXME: this can truncate a wrong length - // should we catch this earlier in the signature of `len` directly? - //written += bytes.gwrite_with(coff_strings.len() as u32, &mut offset, ctx)?; - //written += bytes.gwrite(coff_strings.bytes, &mut offset)?; - } - if let Some(opt_header) = self.header.optional_header { - // Takes care of: - // - export table (.edata) - // - import table (.idata) - // - bound import table - // - import address table - // - delay import tables - // - resource table (.rsrc) - // - exception table (.pdata) - // - base relocation table (.reloc) - // - debug table (.debug) - // - load config table - // - tls table (.tls) - // - architecture (reserved, 0 for now) - // - global ptr is a "empty" data directory (header-only) - // - clr runtime header (.cormeta is object-only) - for (dt_type, dd) in opt_header.data_directories.dirs() { - // - attribute certificate table is a bit special - // its size is not the real size of all certificates - // you can check the parse logic to understand how that works - if dt_type == DataDirectoryType::CertificateTable { - let mut certificate_start = dd.virtual_address.try_into()?; - for certificate in &self.certificates { - bytes.gwrite_with(certificate, &mut certificate_start, ctx)?; - max_offset = max(max_offset, certificate_start); - } - } else { - let offset = dd.offset.ok_or(Self::Error::Malformed( - "Data directory was not read with offset information, cannot rewrite" - .into(), - ))?; - let dd_written = bytes.pwrite(dd.data(&self.bytes)?, offset)?; - max_offset = max(max_offset, offset); - debug!( - "writing {:?} at {} for {} bytes", - dt_type, offset, dd_written - ); - } + + Ok(*offset) + } + + pub fn write_certificates( + &self, + bytes: &mut [u8], + ctx: scroll::Endian, + ) -> Result { + let opt_header = self.header.optional_header.ok_or(error::Error::Malformed( + "This PE binary has no optional header; it is required to write certificates" + .to_string(), + ))?; + let mut max_offset = 0; + + if let Some(certificate_directory) = opt_header.data_directories.get_certificate_table() { + let mut certificate_start = certificate_directory.virtual_address.try_into()?; + for certificate in &self.certificates { + bytes.gwrite_with(certificate, &mut certificate_start, ctx)?; + max_offset = max(certificate_start, max_offset); } } @@ -517,10 +473,12 @@ pub struct Coff<'a> { pub header: header::CoffHeader, /// A list of the sections in this COFF binary pub sections: Vec, - /// The COFF symbol table. - pub symbols: symbol::SymbolTable<'a>, - /// The string table. - pub strings: strtab::Strtab<'a>, + /// The COFF symbol table, they are not guaranteed to exist. + /// For an image, this is expected to be None as COFF debugging information + /// has been deprecated. + pub symbols: Option>, + /// The string table, they don't exist if COFF symbol table does not exist. + pub strings: Option>, } impl<'a> Coff<'a> { @@ -657,7 +615,7 @@ mod tests { #[test] fn string_table_excludes_length() { let coff = Coff::parse(&&COFF_FILE_SINGLE_STRING_IN_STRING_TABLE[..]).unwrap(); - let string_table = coff.strings.to_vec().unwrap(); + let string_table = coff.strings.unwrap().to_vec().unwrap(); assert!(string_table == vec!["ExitProcess"]); } @@ -665,9 +623,10 @@ mod tests { #[test] fn symbol_name_excludes_length() { let coff = Coff::parse(&COFF_FILE_SINGLE_STRING_IN_STRING_TABLE).unwrap(); - let strings = coff.strings; + let strings = coff.strings.unwrap(); let symbols = coff .symbols + .unwrap() .iter() .filter(|(_, name, _)| name.is_none()) .map(|(_, _, sym)| sym.name(&strings).unwrap().to_owned()) diff --git a/src/strtab.rs b/src/strtab.rs index 8a3c568e8..cdb1d38df 100644 --- a/src/strtab.rs +++ b/src/strtab.rs @@ -15,7 +15,7 @@ if_alloc! { /// with your choice of delimiter. Please be careful. pub struct Strtab<'a> { delim: ctx::StrCtx, - pub bytes: &'a [u8], + bytes: &'a [u8], #[cfg(feature = "alloc")] strings: Vec<(usize, &'a str)>, }