Skip to content

Add CRC32 validation when reading zip archive entries#124766

Open
alinpahontu2912 wants to merge 5 commits intodotnet:mainfrom
alinpahontu2912:crc32_validation
Open

Add CRC32 validation when reading zip archive entries#124766
alinpahontu2912 wants to merge 5 commits intodotnet:mainfrom
alinpahontu2912:crc32_validation

Conversation

@alinpahontu2912
Copy link
Member

Add CrcValidatingReadStream to ZipCustomStreams that wraps decompressed entry streams and validates CRC32 checksums as data is read. The stream computes a running CRC32 over all bytes read and compares it against the expected CRC from the zip entry header when the expected number of bytes has been read.

  • Seeking is delegated to the base stream but abandons CRC tracking, since CRC validation requires sequential reading from the start.
  • Flush throws NotSupportedException consistent with read-only streams.
  • Corrupted entries (tampered uncompressed size) now throws InvalidDataException when CRC mismatch is detected.

Update existing corrupted stream tests to expect InvalidDataException from CRC validation instead of silently accepting tampered data.

Add CrcValidatingReadStream to ZipCustomStreams that wraps decompressed
entry streams and validates CRC32 checksums as data is read. The stream
computes a running CRC32 over all bytes read and compares it against the
expected CRC from the zip entry header when the expected number of bytes
has been read.

Key design decisions:
- Seeking is delegated to the base stream but abandons CRC tracking,
  since CRC validation requires sequential reading from the start.
- Flush throws NotSupportedException consistent with read-only streams.
- Corrupted entries (tampered uncompressed size) now correctly throw
  InvalidDataException when CRC mismatch is detected.

Update existing corrupted stream tests to expect InvalidDataException
from CRC validation instead of silently accepting tampered data.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds CRC32 validation when reading ZIP archive entries to detect data corruption. A new CrcValidatingReadStream wrapper class is introduced that computes a running CRC32 checksum as data is read and validates it against the expected CRC from the ZIP entry header once all expected bytes have been read.

Changes:

  • Added CrcValidatingReadStream class that wraps decompressed entry streams and validates CRC32 checksums
  • Updated ZipArchiveEntry to wrap entry streams with CrcValidatingReadStream in read mode
  • Updated corrupted stream tests to expect InvalidDataException from CRC validation instead of silently accepting tampered data
  • Added CrcMismatch resource string for the validation error message

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs Added new CrcValidatingReadStream class that validates CRC32 checksums as data is read from ZIP entry streams
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs Updated OpenInReadMode and OpenInReadModeGetDataCompressor to return CrcValidatingReadStream and wrap decompressed streams with CRC validation
src/libraries/System.IO.Compression/src/Resources/Strings.resx Added CrcMismatch error message for CRC validation failures
src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs Updated corrupted stream tests to expect InvalidDataException when reading tampered entries, simplified tests to focus on CRC validation

@alinpahontu2912
Copy link
Member Author

Should this be marked as breaking change since dealing with corrupted files has now changed ?

@rzikm
Copy link
Member

rzikm commented Feb 25, 2026

Should this be marked as breaking change since dealing with corrupted files has now changed ?

Yes, just like we did for #118577

@alinpahontu2912 alinpahontu2912 added the breaking-change Issue or PR that represents a breaking API or functional change over a previous release. label Feb 25, 2026
@dotnet-policy-service dotnet-policy-service bot added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Feb 25, 2026
@dotnet-policy-service
Copy link
Contributor

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

Copilot AI review requested due to automatic review settings March 3, 2026 12:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 4, 2026 08:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.IO.Compression breaking-change Issue or PR that represents a breaking API or functional change over a previous release. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants