add zip test for blob with negative size#127990
Open
wfurt wants to merge 1 commit intodotnet:mainfrom
Open
Conversation
Contributor
|
Tagging subscribers to this area: @karelz, @dotnet/area-system-io-compression |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a regression/security-hardening test to the System.IO.Compression ZipArchive test suite to exercise parsing of a ZIP64 extra field that encodes an invalid (negative) uncompressed size, ensuring consumption remains safe.
Changes:
- Added a new test covering a ZIP64 extra field with
UncompressedSize = -1. - Added a byte-level ZIP builder helper to generate the malformed archive payload used by the test.
Comment on lines
+2423
to
+2447
| // Attempting to actually read the data must fail safely — either by throwing, | ||
| // or by returning zero bytes (matching the actual stored data). It must NOT | ||
| // crash, hang, allocate based on the negative size, or read past the end of | ||
| // the underlying stream. Defense-in-depth in CrcValidatingReadStream rejects | ||
| // a negative expected length on Open(), so an ArgumentOutOfRangeException or | ||
| // InvalidDataException is the expected outcome here. | ||
| try | ||
| { | ||
| using Stream s = entry.Open(); | ||
| byte[] buffer = new byte[1024]; | ||
| int totalRead = 0; | ||
| int read; | ||
| while ((read = s.Read(buffer, 0, buffer.Length)) > 0) | ||
| { | ||
| totalRead += read; | ||
| // Guard against the negative size being misinterpreted as a huge unsigned | ||
| // length that would let the read loop run forever. | ||
| Assert.True(totalRead <= 1024 * 1024, "Read returned more data than the archive contains."); | ||
| } | ||
| Assert.Equal(0, totalRead); | ||
| } | ||
| catch (Exception ex) when (ex is InvalidDataException or ArgumentOutOfRangeException) | ||
| { | ||
| // Acceptable: downstream code rejects the malformed entry on Open/Read. | ||
| } |
Comment on lines
+2406
to
+2411
| // Validation for IO-021: Zip64 extra field with negative UncompressedSize (-1). | ||
| // The minimal 16-byte Zip64 extra field bypasses the negative-value check in | ||
| // Zip64ExtraField.TryGetZip64BlockFromGenericExtraField (the check only runs after | ||
| // all four fields would have been read). This test verifies that the bypassed | ||
| // negative value does NOT lead to memory corruption, buffer over-read, infinite | ||
| // loop, or other harmful behavior — downstream code handles it safely. |
Comment on lines
+2506
to
+2507
| long negativeUncompressedSize = -1; // 0xFFFFFFFFFFFFFFFF in two's complement | ||
| long negativeCompressedSize = 0; |
Comment on lines
+2533
to
+2536
| // No file data | ||
| long dataOffset = ms.Position; | ||
|
|
||
| // Central Directory File Header |
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.
verifies we throw on malformed zip entry.