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

Prevent zipbomb forged headers attacks #980

Closed
eric-therond-sonarsource opened this issue Sep 22, 2020 · 6 comments
Closed

Prevent zipbomb forged headers attacks #980

eric-therond-sonarsource opened this issue Sep 22, 2020 · 6 comments
Labels
bug Core Issues in jadx-core module
Milestone

Comments

@eric-therond-sonarsource

The isZipBomb method relies on ZipEntry.getSize to retrieve the size of an uncompressed entry, but getSize is unreliable to protect against zipbomb attacks, indeed, it gets the size from zip file headers which can be forged by an attacker.

For instance, this file zipBombForgedHeaders.zip is the same than zipBomb.zip except that the headers have been forged (compressed size of an entry = uncompressed size of an entry):

image

image

The both archives after being uncompressed take several GiB on the disk.

The first zip (ZipBomb.zip) is correctly blocked by jadx but the second not:
./build/jadx/bin/jadx ./zipBombForgedHeaders.zip -d zipbombtest2

image

@eric-therond-sonarsource eric-therond-sonarsource added Core Issues in jadx-core module bug labels Sep 22, 2020
@jpstotz
Copy link
Collaborator

jpstotz commented Sep 22, 2020

Do you know how likely ZIP files with manipulated decompressed size values are for non-ZIP-bomb ZIP files?

Because if the uncompressed size value is usually correct (or zero) it would be possible to wrap the InputStream´ we get from zipFile.getInputStream(entry)` by a FilterStream that counts the number of bytes read and compares it to the expected decompressed size and a fixed maximum value.

@eric-therond-sonarsource
Copy link
Author

I guess no legit ZIP files should have forged headers, most unzip tools will likely treat these files as corrupt or unzip will not work.

Exactly, for a more precise solution see:
https://wiki.sei.cmu.edu/confluence/display/java/IDS04-J.+Safely+extract+files+from+ZipInputStream
or
https://jira.sonarsource.com/browse/RSPEC-5043

@jpstotz
Copy link
Collaborator

jpstotz commented Sep 22, 2020

Sorry but the provided mitigation example in the linked wiki is bad. Just using ZipInputStream may be a solution for Zip bombs, but it has one major disadvantage: it prevents certain ZIP files for being processed at all. This happens if the ZIP files contains one entry that uses STORED compressor. If you encounter such an entry ZipInputStream is unable to read any entry that follows. Such ZIP files are valid. The limitation that such ZIP archives can't be processed using ZipInputStream is by design.

For details see http://commons.apache.org/proper/commons-compress/zip.html#ZipArchiveInputStream_vs_ZipFile or search for the error ZipInputStream: only DEFLATED entries can have EXT descriptor

@skylot
Copy link
Owner

skylot commented Sep 22, 2020

@eric-therond-sonarsource thank you for report and samples!
As soon as the ratio of uncompressed/compressed sizes already checked, I think just limit output bytes count as @jpstotz suggested is enough.
Also, in linked solutions, I notice checks for zip entries count - looks like this also need to be added.

@skylot
Copy link
Owner

skylot commented Sep 22, 2020

I have made a PR #982. Can someone review it? :)

@skylot
Copy link
Owner

skylot commented Sep 27, 2020

PR #982 merged.

@sergey-wowwow can you also check latest commit with zip security enhancements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Core Issues in jadx-core module
Projects
None yet
Development

No branches or pull requests

3 participants