Repository navigation
fix(security): zip limits cannot be bypassed by a header that understates sizes (#1561) - #1639
Merged
Merged
Conversation
…ates sizes (#1561) SafeZipExtractor checks its limits against the uncompressed sizes the zip headers declare. Before archive 4.5.0 that size was only a buffer hint and an entry decoded to whatever its data held, so a crafted header bypassed every limit. Since 4.5.0 an entry is not decoded past its declared size. - Require archive ^4.5.2 (pubspec.lock is not committed, so the constraint is what CI and release builds resolve against). - SafeZipExtractor.readEntry reads an entry's bytes and reports the refusal as a SafeZipException; the local installer, the HTTP marketplace and the updater extract through it. - Tests: a zip whose headers claim 100 bytes for 64 KiB passes the header checks and is refused when read, from bytes and from a file.
…mits # Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1561.
What was wrong
SafeZipExtractor._validateArchivechecks the per-entry, total and ratio limits againstentry.size, the uncompressed size the zip header declares. Up toarchive4.4.x that value was only a buffer hint, so a header claiming 1 KB could inflate to gigabytes onfile.content.What changes
archive4.5.0 closed this in the library: "A zip entry is no longer decoded past the uncompressed size the archive gives for it, soArchiveFile.sizecan be checked before reading." Decoding past it throwsArchiveException(LimitedOutputStream/decodeLimited). Since the declared sizes are exactly what_validateArchivelimits, actual output per entry and in total is now bounded by the checked limits. That is the bounded streaming inflate the issue asks for, done in the decoder, so there is no second inflater of our own.archive: ^4.5.2(was^4.2.0).pubspec.lockis gitignored, so the constraint is the only thing keeping CI and release builds off a vulnerable 4.2–4.4.SafeZipExtractor.readEntry(entry)returns the bytes and turns the decoder's refusal into aSafeZipException("larger than its header declares"). The doc comment states the dependency on 4.5+.HttpMarketplaceRepository.installand the updater'sextractZipSecurelyread entries through it and report it asMarketplaceException/AppUpdaterException, like the other limit errors.Tests
A zip of 64 KiB of zeros whose local and central headers are rewritten to claim 100 bytes:
decodeByteswith the tight test limits (that is the bypass);readEntrythrowsSafeZipException, both from bytes and throughreadAndDecodeFile.An honest entry still reads back. On
archive4.4 the lying-zip tests would fail:contentreturns the full 64 KiB.Not run locally; CI decides.