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

Iterating import directory entry stops at a (completely) null iterm #385

Merged
merged 1 commit into from
Sep 28, 2021

Conversation

daladim
Copy link
Contributor

@daladim daladim commented Sep 27, 2021

Currently, the iterator for PE imports stop at the first item where original_first_thunk == 0. However, we should stop at a (totally) null item, as (more-or-less clearly) stated in the PE file specification

@philipc
Copy link
Contributor

philipc commented Sep 27, 2021

Ok, but in winnt.h, the first field is documented as "0 for terminating null import descriptor". Do you have a file for which this change matters?

@daladim
Copy link
Contributor Author

daladim commented Sep 27, 2021

Hi, thanks for your feedback

I have seen such behaviour on malware samples.
For instance, there is a sample that you can download here : https://bazaar.abuse.ch/sample/db665f26dbc4ca92d326f2cb98faafb9e84d404346b201cd88bec91ce4206bb2/

The import directory is at file offset 0x12591C. Here are the data

00 00 00 00   00 00 00 00   00 00 00 00   00 B6 8F 12   00 20 7D 12  // first DLL (KERNEL32.DLL)
00 00 00 00   00 00 00 00   00 00 00 00   00 00 92 12   00 B0 80 12  // second DLL (GDI32.DLL)
...

The pefile Python library and the CFF explorer tool for instance agree on these imports being correct.

To be sure (and without executing a malware on my machine), I have tried changing these fields on a legitimate PE file: in the import directory of this file on disk, I have set every field but "first_thunk" to 0, and the PE still runs fine. I have attached this file, it was based on https://github.com/corkami/pics/blob/master/binary/pe101/pe101.pdf

pe101-simple - with zeroed original_first_thunks.zip

@philipc
Copy link
Contributor

philipc commented Sep 28, 2021

Thanks for investigating that. This will need a followup so that we can successfully parse these imports though. Currently we only try original_first_thunk, but if that is zero then we need to fall back to first_thunk.

@philipc philipc merged commit 4db37ab into gimli-rs:master Sep 28, 2021
@daladim
Copy link
Contributor Author

daladim commented Sep 28, 2021

Oh right, I forgot about PeFile::imports, I'll take care of it

@philipc
Copy link
Contributor

philipc commented Sep 28, 2021

readobj needs fixing too, and maybe we should add a new method somewhere for this logic.

@daladim daladim mentioned this pull request Sep 28, 2021
@daladim
Copy link
Contributor Author

daladim commented Sep 28, 2021

OK, I've proposed a first part in #386, but I'm afraid I won't be a big help on the readobj part

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

Successfully merging this pull request may close these issues.

None yet

2 participants