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

@@ -262,14 +262,24 @@ impl CoffHeader {
     }
 
     /// Return the COFF symbol table.
-    pub fn symbols<'a>(&self, bytes: &'a [u8]) -> error::Result<symbol::SymbolTable<'a>> {
+    pub fn symbols<'a>(&self, bytes: &'a [u8]) -> error::Result<Option<symbol::SymbolTable<'a>>> {
         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<strtab::Strtab<'a>> {
+    pub fn strings<'a>(&self, bytes: &'a [u8]) -> error::Result<Option<strtab::Strtab<'a>>> {
+        // > 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)?))
     }
 }
 

src/pe/mod.rs

@@ -393,19 +393,19 @@ impl<'a> ctx::TryIntoCtx<scroll::Endian> for PE<'a> {
                     ))
                 })?;
             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(symbols) = self.header.coff_header.symbols(self.bytes)? {
+                bytes.gwrite_with(symbols, &mut offset, ctx)?;
+                let coff_strings =
+                    self.header
+                        .coff_header
+                        .strings(self.bytes)?
+                        .ok_or(Self::Error::Malformed(
+                            "Presence of symbols implies presence of COFF strings".into(),
+                        ))?;
+                bytes.gwrite_with(coff_strings.len() as u32, &mut offset, ctx)?;
+                bytes.gwrite(coff_strings.bytes, &mut offset)?;
+                max_offset = max(offset, max_offset);
+            }
         }
         if let Some(opt_header) = self.header.optional_header {
             // Takes care of:
@@ -459,10 +459,12 @@ pub struct Coff<'a> {
     pub header: header::CoffHeader,
     /// A list of the sections in this COFF binary
     pub sections: Vec<section_table::SectionTable>,
-    /// 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<symbol::SymbolTable<'a>>,
+    /// The string table, they don't exist if COFF symbol table does not exist.
+    pub strings: Option<strtab::Strtab<'a>>,
 }
 
 impl<'a> Coff<'a> {