Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PE: missing exported symbols in PEs #291

Closed
dureuill opened this issue Dec 1, 2021 · 1 comment
Closed

PE: missing exported symbols in PEs #291

dureuill opened this issue Dec 1, 2021 · 1 comment

Comments

@dureuill
Copy link
Contributor

dureuill commented Dec 1, 2021

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant