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

Fileinfo: junk in PE resources #959

Closed
PeterMatula opened this issue Jun 3, 2021 · 1 comment · Fixed by #974
Closed

Fileinfo: junk in PE resources #959

PeterMatula opened this issue Jun 3, 2021 · 1 comment · Fixed by #974

Comments

@PeterMatula
Copy link
Collaborator

PeterMatula commented Jun 3, 2021

We encountered junk data provided by retdec-fileinfo in PE resource table, namely in resource's type value. E.g. for 097a0f8b8c3f2b90f5360f27da5ef8e5a7406c6f211c8d3e56a1671933508bc1:

i     nameId   type                 typeId   language      lanId    slanId   offset     size       crc32   
-----------------------------------------------------------------------------------------------------------
0     105      \x03\x17\xb2\x04\xd7\x17\xddfF&\xf27u\x16P\xfb\x0d\x9d\xc8\x0a\xbf\xec\x06\xbb\xb9\xae\x97\x03p...

Samples:

  • 097a0f8b8c3f2b90f5360f27da5ef8e5a7406c6f211c8d3e56a1671933508bc1
  • 7004f68f9142737b2d6c144df6b6d165c5db518e4f06a9fb1a93d1ff129cefa3
  • 81060c958ce9be17f25c5d41898912042c3f6ed4ad48299494b6bb0f3c46279e
  • a79f638bc52148c8136962fa438c53f7298a73fa8a559262ae77ed78d0ada143
  • b5bcf9ccf1f75f2291112f532789b6e6772a8882ea3b8978bef29e5a6670ac92
  • de2a70001b945717c7970b12acd8eed902e55d54d61137f997e32a4a82e8ca1a
  • f25e8709a0ce7bbcff4ed708510b0833a1086297023fab5eafd1ed9e9dee0822

Investigate the reasons and try to prevent providing such junk data. The solution would be to either fix a bug causing this (if there is a bug) or reliably detect such cases and prevent them. It is however quite possible that file offsets are in fact pointing at existing "junk" data, try to come up with some solution anyway - e.g. heuristics, sanity checks - analyze and discuss with @PeterMatula.

@HoundThe
Copy link
Member

I looked into this, seems like the directory type name points to junk data. I am not sure if there is a valid reason for it, seems like a simple sanity check on string length could be a viable solution (for example allow only strings that are at most 100 characters long - I wasn't sure with the ideal allowed string length as I am not that familiar with PE Resources, but this particular sanity check is what LIEF does as well and it uses 100 characters cap https://github.com/lief-project/LIEF/blob/d8bca167f81eb588c82c8a9e51d0bf267cd627e3/src/PE/Parser.cpp#L414)

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

Successfully merging a pull request may close this issue.

2 participants