CVE-2019-16892: Validate entry sizes when extracting by jdleesmiller · Pull Request #403 · rubyzip/rubyzip

Update (2019-09-25): Assigned CVE-2019-16892

To extract zip files safely:

• If you upgrade to rubyzip >= 1.3.0 and < 2.0.0, you must:

  1. be sure to check entry.size as illustrated in the README before you call entry.extract, and
  2. set Zip.validate_entry_sizes = true to enable the validation added in this PR.

• If you upgrade to rubyzip >= 2.0.0, you must:

  1. be sure to check entry.size as illustrated in the README before you call entry.extract.
  2. (there is no step 2)

If you are using a recent (not EOL) version of ruby, the upgrade to 2.0.0 should be smooth. See the Changelog for details.

Fix #193 (“Protection Against Zip Bombs”)

Currently there is no way to safely check how much data the Zip::Entry#extract method will actually extract, because the uncompressed size reported in the zip can be spoofed, and the extraction happens in the method where the caller can’t check on its progress.

This can lead to denial of service through disk exhaustion if the input is a zip bomb.

Approach

The approach taken here is based on the validateEntrySizes option for node’s yauzl unzipping library: rubyzip can check that it doesn’t extract (much) more than the expected amount of data, based on the uncompressed size reported in the zip file. That way the caller can check the entry’s uncompressed size before extracting and trust that it is not misleading.

The “much” in “much more” above is because rubyzip writes the data in chunks (currently 32KiB), and it may write up to one chunk more than expected, but that should be negligible, and there will be an error that signals that rubyzip wrote more data than expected.

Impact

Zip files with incorrectly reported uncompressed sizes that worked before will now fail with a Zip::EntrySizeError. Like in yauzl, the safe behaviour is the default but a flag is provided to allow the caller to (dangerously) skip the checks:

Zip.validate_entry_sizes = false

EDIT: Following discussion below, false will be the default in the 1.3 release. We’ll default to true in 2.0.

I have updated the README to say that the caller should be checking the size before extracting. I also reorganised some of the options, since there are now quite a few.

Review

I will aim to leave this open for 1 week for review. CC @simonoff @olleolleolle @hainesr who have been active recently.

It probably merits at least a minor version bump.

Credit

Thanks to the GE Digital Cyber Security Team for providing a proof of concept, which was the basis for the test case.

Thanks to @thejoshwolfe for providing (I think) a good example of how to handle this, in yauzl.