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

Fix the Rich header analysis algorithm #973

Merged
merged 4 commits into from
Jul 9, 2021

Conversation

HoundThe
Copy link
Member

@HoundThe HoundThe commented Jun 21, 2021

This PR aims to fix #960 and to fix #965
The Rich header end is marked by the "Rich" marker somewhere between DOS header/stub and PE header. After the "Rich" follows the XOR key, when XORing the Rich header data, the start can be identified by "DanS" in ASCII and 3 null bytes for padding into a 16-byte paragraph.

Current analysis decrypts the whole thing as it assumes there is no other data than Rich header there and then checks if there is "DanS" at the start. This doesn't behave correctly if there is any other "junk" data before the Rich header as it will decrypt it as well as is the case in #960 and just reports warning about wrong key because it didn't find the "DanS" at the start. I've changed the algorithm so it starts decrypting from the end ("Rich") to the top until it finds the "DanS" that marks the start of the Rich header, this seems to work well for both samples

  • 2acd2ff9c70ba9398221cf2265b2fddaceae3e31a29883594bcce545f02be6a3
  • 7f29a26f830eee42a80a1a35169d9f616ca9823e386316f5eccfe36f90a8fe4b

But this means that the Rich header offset can vary based on the junk data, but this isn't really supported by the current RetDec code structure, so I'll need to edit that so we return the correct offset.

Checklist for PR:

  • Fix the analysis algorithm
  • Propagate correct offset from the Rich header analysis
  • Add test case into the regression test suite

@HoundThe
Copy link
Member Author

This PR is also solution to #965

@PeterMatula PeterMatula merged commit b337c66 into avast:master Jul 9, 2021
PeterMatula added a commit that referenced this pull request Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants