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 upper limit for basic block count #661

Merged
merged 2 commits into from
Jan 19, 2023
Merged

add upper limit for basic block count #661

merged 2 commits into from
Jan 19, 2023

Conversation

mr-tz
Copy link
Collaborator

@mr-tz mr-tz commented Jan 13, 2023

closes #435

Library ID fails and RC4 PRGA matches on ___strgtold12_l. An upped BB count prevents this.

Are there concerns on the upper limit value here?

# should not be too simple
- count(basic blocks): 4 or more
# should not be too simple or too complex
- count(basic blocks): (4, 50)
Copy link
Collaborator

Choose a reason for hiding this comment

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

how did we pick 50? i agree we should use an upper limit, but im leaning towards keeping it as low as possible, to keep the rule tight. thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a pretty random value. One concern is around inlined PRGA code, which likely won't trigger as reliably with this or lower basic block count upper limits.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah thats a good point. i recall that our RC4 rules sometimes FP, so i wonder about the tradeoff of catching the inlined routines versus matching too broadly. do you have a sense for this?

anyways, not a blocker here, but a discussion point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really, the only rough data I have is that out of 5000 random samples encrypt data using RC4 PRGA hit on 834 files (14%) which seems a lot. Manually inspecting 3 samples shows 1 valid hit and 2 library function hits (in IDA) that can be filtered out using the upper BB limit.

@mr-tz mr-tz merged commit 2a997f5 into master Jan 19, 2023
@mr-tz mr-tz deleted the fix/435 branch January 19, 2023 12:11
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.

FPs encrypt-data-using-rc4-prga.yml
2 participants