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.

Sign up for GitHub

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

Jump to bottom

Zip parser: tolerate 2-byte overlap in file entries #561

Merged
merged 2 commits into from
Jul 18, 2022
Merged

Zip parser: tolerate 2-byte overlap in file entries #561

merged 2 commits into from
Jul 18, 2022

Conversation

micahsnyder
Copy link
Contributor

The heuristic to alert on overlapping file entries is detecting some
non-malicious JAR files observed in critical enterprise software.
The goal with overlap detection is to alert on non-recursive zip-
bombs, so this tiny overlap isn't a concern.
We'll allow a 2-byte overlap so we don't alert on such zips.

Ref: CLAM-1763

@micahsnyder
Zip parser: tolerate 2-byte overlap in file entries
afd1eed 
The heuristic to alert on overlapping file entries is detecting some
non-malicious JAR files observed in critical enterprise software.
The goal with overlap detection is to alert on non-recursive zip-
bombs, so this tiny overlap isn't a concern.
We'll allow a 2-byte overlap so we don't alert on such zips.
@micahsnyder micahsnyder added this to the 1.0 milestone Apr 29, 2022
m-sola
m-sola reviewed May 25, 2022
View reviewed changes
Copy link
Contributor

@m-sola m-sola left a comment

Choose a reason for hiding this comment

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

Functionally, confirmed no more alert on provided test file.

I don't love how all of this is on one line. If it's not too much trouble could we break the logic up into something more readable?

@micahsnyder
Copy link
Contributor Author

Functionally, confirmed no more alert on provided test file.

I don't love how all of this is on one line. If it's not too much trouble could we break the logic up into something more readable?

Ok I think I fixed it in a nice way. Please re-review.

m-sola
m-sola approved these changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@m-sola m-sola left a comment

Choose a reason for hiding this comment

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

Updates look good!

@micahsnyder micahsnyder added the 🍒cherry-pick-candidate A PR that should be backported once approved. label Jul 15, 2022
@micahsnyder micahsnyder merged commit c86ce2e into Cisco-Talos:main Jul 18, 2022
@micahsnyder micahsnyder deleted the CLAM-1763-overlapping-files branch July 18, 2022 18:38
This was referenced Jul 18, 2022
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m-sola m-sola m-sola approved these changes

Assignees
No one assigned
Labels
🍒cherry-pick-candidate A PR that should be backported once approved.
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@micahsnyder @m-sola