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

How safe is yauzl #55

Closed
ghost opened this issue Apr 28, 2017 · 3 comments
Closed

How safe is yauzl #55

ghost opened this issue Apr 28, 2017 · 3 comments

Comments

@ghost
Copy link

ghost commented Apr 28, 2017

Directory travel issue
https://github.com/ptoomey3/evilarc
https://labs.neohapsis.com/2009/04/21/directory-traversal-in-archives/

Zip bomb
https://www.reddit.com/r/todayilearned/comments/10yniw/til_there_is_a_zip_bomb_called_42zip_that_is_only/

https://en.wikipedia.org/wiki/Zip_bomb

Is there any option to limit only for zip files not to use tar gz ... and archives files limit ?
What happens when somebody create 5 million dirs and empty files ZIP file ?

@thejoshwolfe
Copy link
Owner

Directory travel issue

This is caught and reported as an error. See readme regarding validateFileName().

Zip bomb

Some kinds of zip bomb attacks are caught and reported as errors. See readme regarding guarding against zip bomb attacks. Some kinds of zip bomb attacks are indistinguishable from legitimate zipfiles, such as a zipfile containing very large files, and yauzl does not report any well-formed zipfile (that is supported) as an error.

Is there any option to limit only for zip files not to use tar gz ... and archives files limit ?

I don't quite understand the question. If you're asking how to avoid using zlib decompression, you can pass {decompress: false} to openReadStream(). If you're asking how to only handle the .zip format and not .tar.gz format, that's already how yauzl works; yauzl only reads the .zip format.

What happens when somebody create 5 million dirs and empty files ZIP file ?

By following the recommendation to pass {lazyEntries: false}, you must ask for each next entry via readEntry(). If you do this naively in a loop, you may end up stuck in a loop reading 5 million entries. But you could also impose a limit on your own loop to abort after too many entries, and you can also check the number of entries before entering the loop via the .entryCount field on the zipfile object.

At any time, you can simply close the zip file when you're done reading from it. You don't have to read every entry first.

In general, yauzl's approach to safety is to treat well-formed zipfiles as correct and malformed zipfiles as an error. There are malformations that yauzl does not check, such as crc32 mismatches, but everything regarding safety is enforced. yauzl exposes sufficient information to clients to enable them to impose limits on resource requirements, but yauzl itself does not impose any such limit.

Safety is one of the most important design goals for yauzl; it was one of two reasons I created the project in first place (the other being correctness). All other zipfile readers for node at the time did not treat these topics with sufficient scrutiny, and yauzl aims to be competitive in this regard. Please let me know if you have any more concerns or questions.

@ghost
Copy link
Author

ghost commented Apr 30, 2017

This tool are awesome I can even check file decompression size and has 1000% more options that node-unzip

4.5 Petabytes - zipbomb http://www.unforgettable.dk/42.zip
error: Error: entry is encrypted, and options.decrypt !== false

My sample Zipbomb protection:

  • add options to check file uncompressed size i.e 1KB compressed => 1TB uncompressed
  • add .on('end') and commented better promises
  • max files processed by lazyEntries: true
  • chroot by validateFileName()

https://gist.github.com/dawjan/081e1ce70117fe806b6c3b4bdf43e377

@thejoshwolfe
Copy link
Owner

Great! I left a comment on your gist regarding validateFileName().

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

No branches or pull requests

1 participant