Fix reading Zip64 end of central directory locator#118239
Merged
rzikm merged 9 commits intodotnet:mainfrom Aug 5, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in reading Zip64 end of central directory locator by adding proper stream length validation and correcting internal signature seeking methods. The changes address issue #117147 where malformed ZIP archives with insufficient data could trigger assertions.
Key changes:
- Adds stream length check before attempting to read Zip64 end of central directory locator
- Fixes SeekBackwardsToSignature methods to respect maxBytesToRead parameter properly
- Corrects calculation of total bytes read and overlap handling in signature seeking
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ZipHelper.cs | Fixed SeekBackwardsToSignature to properly respect maxBytesToRead and calculate overlap correctly |
| ZipHelper.Async.cs | Applied same fixes to async version of SeekBackwardsToSignature method |
| ZipArchive.cs | Added stream length validation and simplified EOCD position tracking |
| ZipArchive.Async.cs | Applied same validation and position tracking fixes to async version |
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.Async.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.Async.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.Async.cs
Outdated
Show resolved
Hide resolved
Contributor
|
Tagging subscribers to this area: @dotnet/area-system-io-compression |
…ZipHelper.Async.cs Co-authored-by: Copilot <[email protected]>
stephentoub
reviewed
Jul 31, 2025
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs
Show resolved
Hide resolved
Member
Author
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Member
|
@MihuBot fuzz ZipArchive |
This was referenced Jul 31, 2025
Open
stephentoub
reviewed
Aug 1, 2025
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs
Show resolved
Hide resolved
rzikm
commented
Aug 4, 2025
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs
Show resolved
Hide resolved
Member
Author
|
I think we are ready for review. |
This was referenced Aug 4, 2025
ericstj
approved these changes
Aug 4, 2025
src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs
Show resolved
Hide resolved
rokonec
approved these changes
Aug 5, 2025
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #117147.
Replaces #118230.
The structure of Zip64 ZIP file is as follows:
The applications reading the ZIP archive are expected to parse it from the end, where presence of ZIP64 fields is indicated by special values in the eocd record.
the problematic input is too short to contain eocd locator, which triggers the assert, this PR adds a stream length check for that.
This PR also fixes some internal methods (SeekBackwardsToSignature(Async)) to now behave as expected. Previous implementation always read at least 4kb even if maxBytesToRead was less. This in turn uncovered a different bug where expected offset of eocd locator was not calculated correctly.
Review of the fuzzing code (ZipArchiveFuzzer) revealed some issues (like disposing the stream between runs and ignoring all exceptions), so this PR fixes improves the fuzzer az well.