m4b / goblin

An impish, cross-platform binary parsing crate, written in Rust
MIT License
1.18k stars 156 forks source link

PE: missing exported symbols in PEs #291

Closed dureuill closed 2 years ago

dureuill commented 2 years ago

Hello, and thank you for making goblin!

I've been trying to use goblin (through symbolic), but I noticed that there is a difference in the number of export symbols reported by goblin vs other tools (eg rabin2).

For instance, taking the binaries uploaded in attachment (from open source projects like VirtualBox), I notice the following differences in reported symbols:

binary | goblin | rabin2 libxml2.dll | 1596 | 1597 VBoxMRXNP.dll | 498 | 504

The missing symbols in goblin look like "legit" symbols, e.g. xmlXPathNAN in libxml2.dll (right between xmlXPathNamespaceURIFunction and xmlXPathNewBoolean), and g_pszRTAssertExpr to g_u32RTAssertLine (right between g_aRTUniUpperRanges and NPAddConnection).

Looking for the root cause, I believe I identified it as being in the function section_read_size in src/pe/utils.rs.

It appears that the computed size is sometimes too small.

Reading from #101, the function was introduced in this PR so that goblin correctly loads quine.exe, so simply reverting is probably not an option.

By introducing the changes in #292, I am able to both find the same number of symbols as rabin2, and still load quine.exe.

However, I'm unsure of the change, since it deviates from the linked reference Stack Overflow post. The rationale for the change is that it makes sense to not take anything smaller than the virtual size, I guess?

EDIT: the changes of #293 appear to fix the problem without having to perform changes I'm unsure of on the computed size.

What do you think?

dureuill commented 2 years ago

Fixed by #293, so I'm closing this!