getsentry / pdb

A parser for Microsoft PDB (Program Database) debugging information
https://docs.rs/pdb/
Apache License 2.0
368 stars 68 forks source link

"Attempt to subtract with overflow" panic when calling `PdbInternalSectionOffset::to_rva` #87

Closed landaire closed 3 years ago

landaire commented 3 years ago

When parsing private symbols in my application I get the following panic:

thread 'main' panicked at 'attempt to subtract with overflow', C:\Users\user\.cargo\registry\src\github.com-1ecc6299db9ec823\pdb-0.6.0\src\omap.rs:451:32
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2\/library\std\src\panicking.rs:483
   1: core::panicking::panic_fmt
             at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2\/library\core\src\panicking.rs:85
   2: core::panicking::panic
             at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2\/library\core\src\panicking.rs:50
   3: pdb::omap::get_virtual_address
             at C:\Users\user\.cargo\registry\src\github.com-1ecc6299db9ec823\pdb-0.6.0\src\omap.rs:451
   4: pdb::common::PdbInternalSectionOffset::to_internal_rva
             at C:\Users\user\.cargo\registry\src\github.com-1ecc6299db9ec823\pdb-0.6.0\src\omap.rs:577
   5: pdb::common::PdbInternalSectionOffset::to_rva
             at C:\Users\user\.cargo\registry\src\github.com-1ecc6299db9ec823\pdb-0.6.0\src\omap.rs:570

get_virtual_address is implemented as follows:

fn get_virtual_address(sections: &[ImageSectionHeader], section: u16, offset: u32) -> Option<u32> {
    let section = sections.get(section as usize - 1)?;
    Some(section.virtual_address + offset)
}

I believe that this would indicate section has a value of 0 in this context but I have not dumped it yet. Unfortunately I cannot provide a reproducible testcase considering the PDB is private. I can however provide any additional metadata required to help debug the issue in full.

willglynn commented 3 years ago

Can you describe the build process which created this PDB? My best information is summarized here and it's… murky.

I understand the problematic PDB is private. Would you be able to use a similar process on a separate codebase to produce a new problematic PDB which could be published?

landaire commented 3 years ago

The PDB was created by Microsoft's internal build system for a component of Windows that I don't personally build myself.

What I did notice is that the "broken" symbols all fit the following criteria:

For this project I don't think I care about resolving any of these addresses so an easy mitigation is to just ignore any symbols whose PdbInternalSectionOffset::section is 0. I also just now read from the PdbInternalSectionOffset docs that a section of 0 is invalid or missing, which makes sense.

willglynn commented 3 years ago

Oh, that's a good point. Does this work?

diff --git a/src/omap.rs b/src/omap.rs
index b7f6026..cfd62ac 100644
--- a/src/omap.rs
+++ b/src/omap.rs
@@ -448,8 +448,9 @@ fn get_section_offset(sections: &[ImageSectionHeader], address: u32) -> Option<(
 }

 fn get_virtual_address(sections: &[ImageSectionHeader], section: u16, offset: u32) -> Option<u32> {
-    let section = sections.get(section as usize - 1)?;
-    Some(section.virtual_address + offset)
+    (section as usize).checked_sub(1)
+        .and_then(|i| sections.get(i))
+        .map(|section| section.virtual_address + offset)
 }

 impl Rva {
landaire commented 3 years ago

I haven't tested but that looks great to me.