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

handling of too small NumberOfRvaAndSizes values #265

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

huettenhain
Copy link

This is a proposed fix for #264.

pefile.py Outdated
@@ -2216,6 +2216,15 @@ def __parse__(self, fname, data, fast_load, is_obj):
self.NT_HEADERS.FILE_HEADER = self.FILE_HEADER
self.NT_HEADERS.OPTIONAL_HEADER = self.OPTIONAL_HEADER

# Detect artificially reduced values of NumberOfRvaAndSizes
#
directory_delta = (self.FILE_HEADER.SizeOfOptionalHeader
Copy link

@nightlark nightlark Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came across a sample where SizeOfOptionalHeader also appeared to be inaccurate -- the optional header (including the data directories array) is clearly 240 (0xf0), but then SizeOfOptionalHeader is 160 (0xa0).

10 of the data directory entries are "missing" (e.g. the data directory array entry is present, but the address and size are both == 0), which is communicated by NumberOfRvaAndSizes being set to 6 (e.g. just a convenient precalculated count of "missing" entries). 10*8 bytes = 80 bytes (or 0x50), which just happens to be the difference between the actual space taken by the optional header and the SizeOfOptionalHeader value (0xf0 - 0xa0 = 0x50).

Copy link

@nightlark nightlark Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think the check you have is right as far as detecting misleading NumberOfRvaAndSizes. Upon closer inspection of the file I was looking at, I think the tools we were using were interpreting data after the optional header as data directory entries.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this is pretty much the conclusion I also reached =).

@huettenhain
Copy link
Author

I have rebased the PR onto the most recent version since there has been some recent support. I'd still like to see this in pefile.

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.

2 participants