Skip to content

Fix ZIP64 stream bounding and WinZip AES read-state corruption in ZIP reader#1253

Merged
adamhathcock merged 8 commits intoreleasefrom
adam/fix-zip645-and-zipzstd
Mar 17, 2026
Merged

Fix ZIP64 stream bounding and WinZip AES read-state corruption in ZIP reader#1253
adamhathcock merged 8 commits intoreleasefrom
adam/fix-zip645-and-zipzstd

Conversation

@adamhathcock
Copy link
Owner

@adamhathcock adamhathcock commented Mar 17, 2026

same as #1247 but against release

This pull request makes several important improvements and bug fixes to the WinZip AES decryption logic in WinzipAesCryptoStream, adds comprehensive tests for this functionality, and enhances the test infrastructure for better isolation and reliability. It also introduces new tests for Zstandard-compressed and Zip64 archives, ensuring broader coverage and correctness.

WinZip AES decryption logic improvements

  • Refactored the keystream handling in WinzipAesCryptoStream to correctly preserve the keystream position across non-aligned reads, both for synchronous and asynchronous operations. This fixes issues where partial reads could corrupt decryption state, especially with buffer sizes not aligned to the AES block size. (WinzipAesCryptoStream.cs, WinzipAesCryptoStream.Async.cs) [1] [2] [3]
  • Updated the internal logic to track the current position in the keystream buffer (_counterOutOffset) and to refill it as needed, ensuring correct XOR operations across read boundaries. (WinzipAesCryptoStream.cs, WinzipAesCryptoStream.Async.cs) [1] [2] [3]

Test coverage enhancements

  • Added a new test suite WinzipAesCryptoStreamTests to verify correct decryption for aligned and non-aligned reads, both sync and async, and to ensure the stream stops at the correct payload length. These tests also verify keystream preservation across chunked reads. (WinzipAesCryptoStreamTests.cs)
  • Improved the base test class to use isolated temporary directories for scratch files, ensuring tests are more reliable and do not interfere with each other. Enhanced cleanup logic with error handling and validation. (TestBase.cs) [1] [2] [3]

Zip archive handling and new tests

  • Added tests for reading mixed Zstandard-compressed and WinZip AES-encrypted ZIP archives, verifying that all entries are correctly handled and decompressed. (ZipArchiveTests.cs) [1] [2]
  • Added a test for reading large Zip64 archives, ensuring that streams are properly bounded to entry lengths and that very large files are handled correctly. (ZipArchiveTests.cs) [1] [2] [3]

Minor bug fixes

  • Cleaned up conditional logic in ZipFilePart to simplify stream handling for certain ZIP header conditions. (ZipFilePart.cs, ZipFilePart.Async.cs) [1] [2]

These changes collectively improve the reliability, correctness, and test coverage of ZIP AES decryption and ZIP archive handling in the library.

@adamhathcock adamhathcock changed the title Adam/fix zip645 and zipzstd Fix ZIP64 stream bounding and WinZip AES read-state corruption in ZIP reader Mar 17, 2026
@adamhathcock adamhathcock marked this pull request as ready for review March 17, 2026 12:17
@adamhathcock adamhathcock merged commit 1bfc11a into release Mar 17, 2026
3 checks passed
@adamhathcock adamhathcock deleted the adam/fix-zip645-and-zipzstd branch March 17, 2026 12:22
@kilo-code-bot
Copy link
Contributor

kilo-code-bot bot commented Mar 17, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (10 files)
  • Directory.Build.props - Added NoWarn suppression for IDE0051
  • src/SharpCompress/Common/Zip/WinzipAesCryptoStream.cs - Refactored keystream handling with _counterOutOffset
  • src/SharpCompress/Common/Zip/WinzipAesCryptoStream.Async.cs - Async refactoring with same approach
  • src/SharpCompress/Common/Zip/ZipFilePart.cs - Fixed Zip64 entry bounding logic
  • src/SharpCompress/Common/Zip/ZipFilePart.Async.cs - Fixed async version
  • tests/SharpCompress.Test/Streams/WinzipAesCryptoStreamTests.cs - New comprehensive crypto stream tests
  • tests/SharpCompress.Test/TestBase.cs - Test infrastructure improvements (per-test temp directories)
  • tests/SharpCompress.Test/Zip/ZipArchiveTests.cs - New Zip64 and Zstandard+WinzipAES tests
  • tests/SharpCompress.Test/Zip/ZipFilePartTests.cs - New ZipFilePart tests
  • tests/TestArchives/Archives/Zip.zstd.WinzipAES.mixed.zip - New test archive

Review Details

The PR implements important bug fixes and test coverage for ZIP handling:

  1. WinZip AES keystream corruption fix: The refactoring from _isFinalBlock boolean to _counterOutOffset integer correctly handles non-aligned buffer reads, preserving keystream position across read boundaries.

  2. Zip64 stream bounding fix: Removed the incorrect || Header.IsZip64 condition that was leaving Zip64 entries unbounded. Now entries with known sizes (including Zip64) are properly wrapped in ReadOnlySubStream.

  3. Test coverage: Comprehensive new tests verify:

    • Aligned and non-aligned read behavior
    • Keystream preservation across chunked reads
    • Correct payload length enforcement
    • Zip64 entry bounding
    • Mixed Zstandard + WinZip AES archives
  4. Test infrastructure: Improved test isolation with per-test temp directories and better cleanup handling.

The code changes follow existing patterns in the codebase and the test coverage is thorough.

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

Successfully merging this pull request may close these issues.

2 participants