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

Add check that the resource file offset is valid #982

Merged
merged 5 commits into from
Aug 25, 2021

Conversation

HoundThe
Copy link
Member

@HoundThe HoundThe commented Jul 5, 2021

Addresses partially #963

Add check that the file offset of a Resource is actually somewhere inside section physical data

Sample that has such invalid offsets:
8a8154c36977c3f47a65c329a6dd93c58b8aff6050a4a748f4c610310daf0d8b

If this fix is wanted, there are few things to consider:

  • This means that offset 0 is considered bad. If 0 offsets could be valid (But I think they are not?) then the feedback about invalid offset should be changed
  • For the offset 0 entries, the hash is calculated, which is unnecessary and could be misleading. This needs to be addressed too before possible merging.

@metthal
Copy link
Member

metthal commented Aug 5, 2021

For the offset 0 entries, the hash is calculated, which is unnecessary and could be misleading. This needs to be addressed too before possible merging.

Good point, could this be please incorporated into these changes?

@HoundThe
Copy link
Member Author

HoundThe commented Aug 6, 2021

I've added a flag that represents the offset validity, so invalid offsets are not exported to the output at all.

src/pelib/ImageLoader.cpp Outdated Show resolved Hide resolved
src/pelib/ImageLoader.cpp Show resolved Hide resolved
src/fileformat/types/resource_table/resource.cpp Outdated Show resolved Hide resolved
@PeterMatula
Copy link
Collaborator

lets run TC tests

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.

3 participants