From d155b1ea51fb9875bcfd4fff8cfa7d148be45288 Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Tue, 18 Aug 2026 12:13:24 +0200 Subject: [PATCH 1/9] check maximum possible compressed size in ziparchive entries --- .../src/Resources/Strings.resx | 3 + .../IO/Compression/ZipArchiveEntry.Async.cs | 1 + .../System/IO/Compression/ZipArchiveEntry.cs | 51 +++++++++++++ .../zip_InvalidParametersAndStrangeFiles.cs | 75 +++++++++++++++++++ .../System.IO.Packaging/tests/Tests.cs | 75 +++++++++++++++++++ 5 files changed, 205 insertions(+) diff --git a/src/libraries/System.IO.Compression/src/Resources/Strings.resx b/src/libraries/System.IO.Compression/src/Resources/Strings.resx index 8f70a3e2dea3d2..e624cad118e81a 100644 --- a/src/libraries/System.IO.Compression/src/Resources/Strings.resx +++ b/src/libraries/System.IO.Compression/src/Resources/Strings.resx @@ -216,6 +216,9 @@ Entries with uncompressed data larger than 2GB are not supported in Update mode. + + The entry's declared uncompressed size ({0}) is implausible given its compressed size ({1}). The archive header may be corrupt or malicious. + End of Central Directory record could not be found. diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs index f352d28b4578f2..5bb91c1795607d 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs @@ -357,6 +357,7 @@ private async Task GetUncompressedDataAsync(CancellationToken canc throw new InvalidDataException(SR.EntryUncompressedSizeTooLargeForUpdateMode); } + ValidateUncompressedSizeIsPlausible(); _storedUncompressedData = new MemoryStream((int)_uncompressedSize); diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs index 4dd6b616f35bb6..a5353d2d6d5f74 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs @@ -596,6 +596,23 @@ private string DecodeEntryString(byte[] entryStringBytes) // will not work in a 32-bit process. private static readonly bool s_allowLargeZipArchiveEntriesInUpdateMode = IntPtr.Size > 4; + // A DEFLATE length/distance pair can encode a back-reference match of at most 258 bytes + // (RFC 1951), so a single, non-nested DEFLATE stream cannot legitimately expand by more + // than about 1032x (258 bytes of match per few bits of compressed data). This bounds how + // large a declared uncompressed size can plausibly be relative to the actual compressed + // data present in the archive. + private const long MaxDeflateExpansionRatio = 1032; + + // Deflate64 extends the maximum match length from 258 up to 65538 bytes (see + // InflaterManaged/OutputWindow), so its worst-case expansion ratio scales accordingly. + // 256x is a round, conservative multiplier that comfortably covers the ~254x increase + // in maximum match length without relying on an exact derivation. + private const long MaxDeflate64ExpansionRatio = MaxDeflateExpansionRatio * 256; + + // A small additive allowance so that tiny, legitimate entries (where fixed per-stream + // overhead dominates the ratio math) are never rejected. + private const long MinPlausibleUncompressedSizeAllowance = 4096; + internal bool EverOpenedForWrite => _everOpenedForWrite; internal long GetOffsetOfCompressedData() @@ -654,6 +671,38 @@ internal void ReadEncryptionSaltIfNeeded() } } + // Validates that this entry's declared _uncompressedSize is plausible given its _compressedSize, + // i.e. that it could actually have been produced by decompressing the bytes physically present + // in the archive. Without this check, an entry can declare an uncompressed size wildly out of + // proportion to its (small) compressed size, causing GetUncompressedData/GetUncompressedDataAsync + // to eagerly allocate a MemoryStream sized to that untrusted value before any decompression is + // attempted - e.g. a few hundred bytes of archive claiming multiple gigabytes of uncompressed + // content. Only applies to entries whose data originates from the archive being read; entries + // created fresh in this session have no untrusted header to validate. + private void ValidateUncompressedSizeIsPlausible() + { + if (!_originallyInArchive || CompressionMethod == ZipCompressionMethod.Stored) + { + // Stored entries cannot expand: their uncompressed size must equal their compressed size. + return; + } + + long maxExpansionRatio = CompressionMethod == ZipCompressionMethod.Deflate64 + ? MaxDeflate64ExpansionRatio + : MaxDeflateExpansionRatio; + + if (_uncompressedSize > MinPlausibleUncompressedSizeAllowance) + { + // Divide rather than multiply to avoid any risk of overflow. + long minPlausibleCompressedSize = (_uncompressedSize - MinPlausibleUncompressedSizeAllowance) / maxExpansionRatio; + if (_compressedSize < minPlausibleCompressedSize) + { + _currentlyOpenForWrite = false; + throw new InvalidDataException(SR.Format(SR.EntryUncompressedSizeImplausible, _uncompressedSize, _compressedSize)); + } + } + } + private MemoryStream GetUncompressedData(ReadOnlySpan password = default) { if (_storedUncompressedData == null) @@ -669,6 +718,8 @@ private MemoryStream GetUncompressedData(ReadOnlySpan password = default) throw new InvalidDataException(SR.EntryUncompressedSizeTooLargeForUpdateMode); } + ValidateUncompressedSizeIsPlausible(); + _storedUncompressedData = new MemoryStream((int)_uncompressedSize); if (_originallyInArchive) diff --git a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs index 43ce9d2f78b554..ce7c517fd7ca91 100644 --- a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs +++ b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs @@ -449,6 +449,81 @@ public static async Task ZipArchiveEntry_OpenInUpdateMode_UncompressedSizeGreate await DisposeZipArchive(async, archive); } + [Theory] + [MemberData(nameof(Get_Booleans_Data))] + public static async Task ZipArchiveEntry_OpenInUpdateMode_UncompressedSizeImplausibleGivenCompressedSize_ThrowsInvalidData(bool async) + { + // A small compressed entry cannot legitimately decompress to a wildly larger size: a single + // DEFLATE stream can expand by, at most, a few orders of magnitude (see MaxDeflateExpansionRatio). + // If the declared _uncompressedSize is implausible relative to the actual _compressedSize present + // in the archive (as could happen with a spoofed/corrupt header), the entry must be rejected up + // front in Update mode, before a MemoryStream sized to the untrusted declared value is allocated. + byte[] payload = [0xCA, 0xFE, 0xBA, 0xBE, 0xDE, 0xAD, 0xBE, 0xEF]; + MemoryStream stream = new MemoryStream(); + + // Use an actual Deflate-compressed entry (not Stored): the plausibility check only + // applies to compression methods that can expand data, and Stored cannot. + ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Create, leaveOpen: true); + ZipArchiveEntry entry = archive.CreateEntry("entry.bin", CompressionLevel.Optimal); + Stream entryStream = await OpenEntryStream(async, entry); + await entryStream.WriteAsync(payload); + await DisposeStream(async, entryStream); + await DisposeZipArchive(async, archive); + + stream.Position = 0; + archive = await CreateZipArchive(async, stream, ZipArchiveMode.Update, leaveOpen: true); + entry = archive.GetEntry("entry.bin"); + + // The entry's actual compressed size is a handful of bytes; claim a 50 MB uncompressed size, + // which is many orders of magnitude beyond what that compressed data could plausibly produce. + FieldInfo uncompressedSizeField = typeof(ZipArchiveEntry).GetField("_uncompressedSize", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(uncompressedSizeField); + uncompressedSizeField.SetValue(entry, 50_000_000L); + + if (async) + { + await Assert.ThrowsAsync(() => entry.OpenAsync()); + } + else + { + Assert.Throws(() => entry.Open()); + } + + await DisposeZipArchive(async, archive); + } + + [Theory] + [MemberData(nameof(Get_Booleans_Data))] + public static async Task ZipArchiveEntry_OpenInUpdateMode_HighButPlausibleCompressionRatio_OpensSuccessfully(bool async) + { + // Legitimately compressible content (a highly repetitive payload) can have a large, but + // plausible, expansion ratio. This must continue to open successfully in Update mode. + byte[] payload = new byte[1_000_000]; + Array.Fill(payload, (byte)'A'); + MemoryStream stream = new MemoryStream(); + + ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Create, leaveOpen: true); + ZipArchiveEntry entry = archive.CreateEntry("entry.bin", CompressionLevel.Optimal); + Stream entryStream = await OpenEntryStream(async, entry); + await entryStream.WriteAsync(payload); + await DisposeStream(async, entryStream); + await DisposeZipArchive(async, archive); + + stream.Position = 0; + archive = await CreateZipArchive(async, stream, ZipArchiveMode.Update, leaveOpen: true); + entry = archive.GetEntry("entry.bin"); + + using (MemoryStream ms = new MemoryStream()) + { + Stream source = await OpenEntryStream(async, entry); + await source.CopyToAsync(ms); + Assert.Equal(payload.Length, ms.Length); + await DisposeStream(async, source); + } + + await DisposeZipArchive(async, archive); + } + [Theory] [MemberData(nameof(Get_Booleans_Data))] public static async Task UnseekableVeryLargeArchive_DataDescriptor_Read_Zip64(bool async) diff --git a/src/libraries/System.IO.Packaging/tests/Tests.cs b/src/libraries/System.IO.Packaging/tests/Tests.cs index 7988907382fbdd..c9dae28450bef6 100644 --- a/src/libraries/System.IO.Packaging/tests/Tests.cs +++ b/src/libraries/System.IO.Packaging/tests/Tests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Buffers.Binary; using System.Linq; using System.Runtime.CompilerServices; using System.Text; @@ -101,6 +102,80 @@ public void GetStreamCreate_OverwritesExistingPartContentWithoutLeftoverBytes(Fi } } + [Fact] + public void Open_ContentTypesEntryWithImplausibleDeclaredUncompressedSize_ThrowsInvalidDataException() + { + // Regression test: Package.Open's default ReadWrite access automatically parses the mandatory + // [Content_Types].xml part during Open(). If that entry's declared (but untrusted) uncompressed + // size is implausible relative to its actual compressed size - as would happen with a corrupt + // or maliciously crafted header - Open() must reject the archive instead of eagerly allocating + // a buffer sized to the attacker-controlled value. + FileInfo file = GetTempFileInfoWithExtension(".zip"); + + using (Package package = Package.Open(file.FullName, FileMode.Create, FileAccess.ReadWrite)) + { + // The [Content_Types].xml entry inherits its compression level from the first part added + // to the package, so a compressed option (rather than the default NotCompressed/Stored) is + // required here for the entry to be Deflate-compressed and therefore subject to the + // uncompressed-size plausibility check under test. + PackagePart part = package.CreatePart( + PackUriHelper.CreatePartUri(new Uri("MyFile.xml", UriKind.Relative)), + Mime_MediaTypeNames_Text_Xml, + CompressionOption.Normal); + using Stream s = part.GetStream(FileMode.Create, FileAccess.Write); + byte[] content = Encoding.UTF8.GetBytes(s_DocumentXml); + s.Write(content, 0, content.Length); + } + + byte[] archiveBytes = File.ReadAllBytes(file.FullName); + PatchContentTypesUncompressedSize(archiveBytes, implausibleUncompressedSize: 500_000_000); + File.WriteAllBytes(file.FullName, archiveBytes); + + Assert.Throws(() => Package.Open(file.FullName, FileMode.Open, FileAccess.ReadWrite)); + } + + // Patches the declared uncompressed size field (in both the local file header and the central + // directory record) for the "[Content_Types].xml" entry within a raw, in-memory zip byte array. + private static void PatchContentTypesUncompressedSize(byte[] archiveBytes, uint implausibleUncompressedSize) + { + const string EntryName = "[Content_Types].xml"; + byte[] nameBytes = Encoding.ASCII.GetBytes(EntryName); + ReadOnlySpan localHeaderSignature = [0x50, 0x4B, 0x03, 0x04]; + ReadOnlySpan centralDirectorySignature = [0x50, 0x4B, 0x01, 0x02]; + + int patchedCount = 0; + int searchStart = 0; + Span archiveSpan = archiveBytes; + while (true) + { + int nameIndex = archiveSpan.Slice(searchStart).IndexOf((ReadOnlySpan)nameBytes); + if (nameIndex < 0) + { + break; + } + nameIndex += searchStart; + searchStart = nameIndex + 1; + + // Local file header: fixed 30-byte header immediately precedes the file name; the + // uncompressed size field is the 4 bytes located 8 bytes before the file name starts. + if (nameIndex >= 30 && archiveSpan.Slice(nameIndex - 30, 4).SequenceEqual(localHeaderSignature)) + { + BinaryPrimitives.WriteUInt32LittleEndian(archiveSpan.Slice(nameIndex - 8, 4), implausibleUncompressedSize); + patchedCount++; + } + // Central directory file header: fixed 46-byte header immediately precedes the file name; + // the uncompressed size field is the 4 bytes located 22 bytes before the file name starts. + else if (nameIndex >= 46 && archiveSpan.Slice(nameIndex - 46, 4).SequenceEqual(centralDirectorySignature)) + { + BinaryPrimitives.WriteUInt32LittleEndian(archiveSpan.Slice(nameIndex - 22, 4), implausibleUncompressedSize); + patchedCount++; + } + } + + // Sanity check: both the local header and central directory copies must have been found and patched. + Assert.Equal(2, patchedCount); + } + [Fact] public void T201_FileFormatException() { From 6dd2debae12f87e2b5c7dd95304e832b5dabe980 Mon Sep 17 00:00:00 2001 From: Stefan-Alin Pahontu <56953855+alinpahontu2912@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:53:27 +0200 Subject: [PATCH 2/9] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/Resources/Strings.resx | 6 +++--- .../src/System/IO/Compression/ZipArchiveEntry.cs | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.IO.Compression/src/Resources/Strings.resx b/src/libraries/System.IO.Compression/src/Resources/Strings.resx index e624cad118e81a..45df0652a5b9b0 100644 --- a/src/libraries/System.IO.Compression/src/Resources/Strings.resx +++ b/src/libraries/System.IO.Compression/src/Resources/Strings.resx @@ -213,12 +213,12 @@ Entries larger than 4GB are not supported in Update mode. - - Entries with uncompressed data larger than 2GB are not supported in Update mode. - The entry's declared uncompressed size ({0}) is implausible given its compressed size ({1}). The archive header may be corrupt or malicious. + + Entries with uncompressed data larger than 2GB are not supported in Update mode. + End of Central Directory record could not be found. diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs index a5353d2d6d5f74..05f678df301a04 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs @@ -681,9 +681,19 @@ internal void ReadEncryptionSaltIfNeeded() // created fresh in this session have no untrusted header to validate. private void ValidateUncompressedSizeIsPlausible() { - if (!_originallyInArchive || CompressionMethod == ZipCompressionMethod.Stored) + if (!_originallyInArchive) { - // Stored entries cannot expand: their uncompressed size must equal their compressed size. + return; + } + + if (CompressionMethod == ZipCompressionMethod.Stored) + { + if (_uncompressedSize != _compressedSize) + { + _currentlyOpenForWrite = false; + throw new InvalidDataException(SR.Format(SR.EntryUncompressedSizeImplausible, _uncompressedSize, _compressedSize)); + } + return; } From 7c3d1e7f1ce1e743fb8f49dd82558f023a5a5671 Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Tue, 18 Aug 2026 14:32:58 +0200 Subject: [PATCH 3/9] add compressedsize check to encrypted entries too --- .../IO/Compression/ZipArchiveEntry.Async.cs | 2 + .../zip_InvalidParametersAndStrangeFiles.cs | 37 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs index 5bb91c1795607d..b795da94b6c5c8 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs @@ -605,6 +605,8 @@ private async Task StoreDecryptedDataForUpdateAsync(Stream decryptedStre throw new InvalidDataException(SR.EntryTooLarge); } + ValidateUncompressedSizeIsPlausible(); + _storedUncompressedData = new MemoryStream((int)_uncompressedSize); Stream decompressed = BuildDecompressionPipeline(decryptedStream); diff --git a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs index ce7c517fd7ca91..96e18323f6729a 100644 --- a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs +++ b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs @@ -524,6 +524,43 @@ public static async Task ZipArchiveEntry_OpenInUpdateMode_HighButPlausibleCompre await DisposeZipArchive(async, archive); } + [Theory] + [MemberData(nameof(Get_Booleans_Data))] + [SkipOnPlatform(TestPlatforms.Browser, "WinZip AES encryption is not supported on browser.")] + public static async Task ZipArchiveEntry_OpenWithPasswordInUpdateMode_UncompressedSizeImplausibleGivenCompressedSize_ThrowsInvalidData(bool async) + { + // The encrypted re-open-for-update path decrypts and decompresses into a MemoryStream sized to + // the declared uncompressed size. In the async implementation this happens via a separate code + // path (StoreDecryptedDataForUpdateAsync) from the unencrypted GetUncompressedDataAsync path, + // while the sync implementation shares GetUncompressedData(password) with the unencrypted path. + // Both must apply the same uncompressed-size plausibility check. + const string Password = "S3cur3P@ssw0rd"; + byte[] payload = [0xCA, 0xFE, 0xBA, 0xBE, 0xDE, 0xAD, 0xBE, 0xEF]; + MemoryStream stream = new MemoryStream(); + + ZipArchive createArchive = await CreateZipArchive(async, stream, ZipArchiveMode.Create, leaveOpen: true); + ZipArchiveEntry createEntry = createArchive.CreateEntry("entry.bin", CompressionLevel.Optimal, Password, ZipEncryptionMethod.Aes256); + Stream entryStream = await OpenEntryStream(async, createEntry, Password); + await entryStream.WriteAsync(payload); + await DisposeStream(async, entryStream); + await DisposeZipArchive(async, createArchive); + + stream.Position = 0; + ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Update, leaveOpen: true); + ZipArchiveEntry entry = archive.GetEntry("entry.bin"); + Assert.True(entry.IsEncrypted); + + // The entry's actual compressed size is a handful of bytes; claim a 50 MB uncompressed size, + // which is many orders of magnitude beyond what that compressed data could plausibly produce. + FieldInfo uncompressedSizeField = typeof(ZipArchiveEntry).GetField("_uncompressedSize", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(uncompressedSizeField); + uncompressedSizeField.SetValue(entry, 50_000_000L); + + await Assert.ThrowsAsync(() => OpenEntryStream(async, entry, Password)); + + await DisposeZipArchive(async, archive); + } + [Theory] [MemberData(nameof(Get_Booleans_Data))] public static async Task UnseekableVeryLargeArchive_DataDescriptor_Read_Zip64(bool async) From 275f1c58a63166bff9a553c234d0aa710b7b7665 Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Thu, 20 Aug 2026 15:03:28 +0200 Subject: [PATCH 4/9] skip test on netfx --- src/libraries/System.IO.Packaging/tests/Tests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.IO.Packaging/tests/Tests.cs b/src/libraries/System.IO.Packaging/tests/Tests.cs index c9dae28450bef6..fa54a7cdc2f3ea 100644 --- a/src/libraries/System.IO.Packaging/tests/Tests.cs +++ b/src/libraries/System.IO.Packaging/tests/Tests.cs @@ -103,6 +103,7 @@ public void GetStreamCreate_OverwritesExistingPartContentWithoutLeftoverBytes(Fi } [Fact] + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Desktop's built-in System.IO.Packaging implementation wraps the size-mismatch error in a FileFormatException instead of throwing InvalidDataException directly")] public void Open_ContentTypesEntryWithImplausibleDeclaredUncompressedSize_ThrowsInvalidDataException() { // Regression test: Package.Open's default ReadWrite access automatically parses the mandatory From da2020ab50928884cf26d27a4f0ec846371e487a Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Tue, 1 Sep 2026 10:50:12 +0200 Subject: [PATCH 5/9] cap the alloation size at minimum between compressed and uncompressed entry size --- .../IO/Compression/ZipArchiveEntry.Async.cs | 12 +++- .../System/IO/Compression/ZipArchiveEntry.cs | 6 +- .../zip_InvalidParametersAndStrangeFiles.cs | 60 +++++++++++++++++++ 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs index b795da94b6c5c8..61bc6398c705b2 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs @@ -359,7 +359,11 @@ private async Task GetUncompressedDataAsync(CancellationToken canc ValidateUncompressedSizeIsPlausible(); - _storedUncompressedData = new MemoryStream((int)_uncompressedSize); + // _compressedSize is already validated against the archive's real length (above), so + // capping capacity to it avoids a huge allocation from a small archive; the stream + // still grows normally if the entry decompresses larger than this. + int initialCapacity = (int)Math.Min(_uncompressedSize, _compressedSize); + _storedUncompressedData = new MemoryStream(initialCapacity); if (_originallyInArchive) { @@ -607,7 +611,11 @@ private async Task StoreDecryptedDataForUpdateAsync(Stream decryptedStre ValidateUncompressedSizeIsPlausible(); - _storedUncompressedData = new MemoryStream((int)_uncompressedSize); + // _compressedSize is already validated against the archive's real length (above), so + // capping capacity to it avoids a huge allocation from a small archive; the stream + // still grows normally if the entry decompresses larger than this. + int initialCapacity = (int)Math.Min(_uncompressedSize, _compressedSize); + _storedUncompressedData = new MemoryStream(initialCapacity); Stream decompressed = BuildDecompressionPipeline(decryptedStream); diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs index 05f678df301a04..7d9864b5553280 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs @@ -730,7 +730,11 @@ private MemoryStream GetUncompressedData(ReadOnlySpan password = default) ValidateUncompressedSizeIsPlausible(); - _storedUncompressedData = new MemoryStream((int)_uncompressedSize); + // _compressedSize is already validated against the archive's real length (above), so + // capping capacity to it avoids a huge allocation from a small archive; the stream + // still grows normally if the entry decompresses larger than this. + int initialCapacity = (int)Math.Min(_uncompressedSize, _compressedSize); + _storedUncompressedData = new MemoryStream(initialCapacity); if (_originallyInArchive) { diff --git a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs index 96e18323f6729a..a3a6fb5b7ac8f1 100644 --- a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs +++ b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs @@ -524,6 +524,66 @@ public static async Task ZipArchiveEntry_OpenInUpdateMode_HighButPlausibleCompre await DisposeZipArchive(async, archive); } + [Theory] + [MemberData(nameof(Get_Booleans_Data))] + public static async Task ZipArchiveEntry_OpenInUpdateMode_LargePlausibleClaimedSize_DoesNotPreallocateFullClaimedSize(bool async) + { + // A mathematically-plausible but untrue _uncompressedSize claim should not cause the initial + // MemoryStream to be pre-allocated to that claimed size; it should be capped at + // _compressedSize instead, and grow normally only as real data is copied in. + // + // The payload is incompressible so _compressedSize stays close to its length, keeping + // IsOpenableFinalVerifications satisfied while we lie about _uncompressedSize below. + byte[] payload = new byte[900_000]; + new Random(42).NextBytes(payload); + MemoryStream stream = new MemoryStream(); + + ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Create, leaveOpen: true); + ZipArchiveEntry entry = archive.CreateEntry("entry.bin", CompressionLevel.Fastest); + Stream entryStream = await OpenEntryStream(async, entry); + await entryStream.WriteAsync(payload); + await DisposeStream(async, entryStream); + await DisposeZipArchive(async, archive); + + stream.Position = 0; + archive = await CreateZipArchive(async, stream, ZipArchiveMode.Update, leaveOpen: true); + entry = archive.GetEntry("entry.bin"); + + // The real _compressedSize (~900,000 bytes) makes a 500 MB claim mathematically plausible + // under ValidateUncompressedSizeIsPlausible, even though the real content is far smaller. + FieldInfo uncompressedSizeField = typeof(ZipArchiveEntry).GetField("_uncompressedSize", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(uncompressedSizeField); + uncompressedSizeField.SetValue(entry, 500_000_000L); + + FieldInfo compressedSizeField = typeof(ZipArchiveEntry).GetField("_compressedSize", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(compressedSizeField); + long realCompressedSize = Assert.IsType(compressedSizeField.GetValue(entry)); + + Stream source = await OpenEntryStream(async, entry); + + using (MemoryStream ms = new MemoryStream()) + { + await source.CopyToAsync(ms); + Assert.Equal(payload.Length, ms.Length); + Assert.Equal(payload, ms.ToArray()); + } + + await DisposeStream(async, source); + + FieldInfo storedUncompressedDataField = typeof(ZipArchiveEntry).GetField("_storedUncompressedData", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(storedUncompressedDataField); + MemoryStream storedUncompressedData = Assert.IsType(storedUncompressedDataField.GetValue(entry)); + + // The real capacity should track the physically-supplied compressed size, not the untrusted + // 500 MB claim. Bound this against realCompressedSize rather than asserting exact equality: + // exact equality would depend on Deflate never compressing this payload below its original + // length, which isn't a guaranteed property of the compressor. + Assert.True(storedUncompressedData.Capacity <= realCompressedSize * 2); + Assert.True(storedUncompressedData.Capacity < 500_000_000 / 10); + + await DisposeZipArchive(async, archive); + } + [Theory] [MemberData(nameof(Get_Booleans_Data))] [SkipOnPlatform(TestPlatforms.Browser, "WinZip AES encryption is not supported on browser.")] From 7d24f85a32f76d01f7acbe1f6e11e476250c1220 Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Tue, 1 Sep 2026 16:45:06 +0200 Subject: [PATCH 6/9] cap size of package part and undo zip changes --- .../src/Resources/Strings.resx | 3 - .../IO/Compression/ZipArchiveEntry.Async.cs | 15 +- .../System/IO/Compression/ZipArchiveEntry.cs | 67 +------ .../zip_InvalidParametersAndStrangeFiles.cs | 172 ------------------ .../src/Resources/Strings.resx | 3 + .../src/System/IO/Packaging/ZipPackage.cs | 16 ++ .../System.IO.Packaging/tests/Tests.cs | 28 ++- 7 files changed, 35 insertions(+), 269 deletions(-) diff --git a/src/libraries/System.IO.Compression/src/Resources/Strings.resx b/src/libraries/System.IO.Compression/src/Resources/Strings.resx index 45df0652a5b9b0..8f70a3e2dea3d2 100644 --- a/src/libraries/System.IO.Compression/src/Resources/Strings.resx +++ b/src/libraries/System.IO.Compression/src/Resources/Strings.resx @@ -213,9 +213,6 @@ Entries larger than 4GB are not supported in Update mode. - - The entry's declared uncompressed size ({0}) is implausible given its compressed size ({1}). The archive header may be corrupt or malicious. - Entries with uncompressed data larger than 2GB are not supported in Update mode. diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs index 61bc6398c705b2..f352d28b4578f2 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs @@ -357,13 +357,8 @@ private async Task GetUncompressedDataAsync(CancellationToken canc throw new InvalidDataException(SR.EntryUncompressedSizeTooLargeForUpdateMode); } - ValidateUncompressedSizeIsPlausible(); - // _compressedSize is already validated against the archive's real length (above), so - // capping capacity to it avoids a huge allocation from a small archive; the stream - // still grows normally if the entry decompresses larger than this. - int initialCapacity = (int)Math.Min(_uncompressedSize, _compressedSize); - _storedUncompressedData = new MemoryStream(initialCapacity); + _storedUncompressedData = new MemoryStream((int)_uncompressedSize); if (_originallyInArchive) { @@ -609,13 +604,7 @@ private async Task StoreDecryptedDataForUpdateAsync(Stream decryptedStre throw new InvalidDataException(SR.EntryTooLarge); } - ValidateUncompressedSizeIsPlausible(); - - // _compressedSize is already validated against the archive's real length (above), so - // capping capacity to it avoids a huge allocation from a small archive; the stream - // still grows normally if the entry decompresses larger than this. - int initialCapacity = (int)Math.Min(_uncompressedSize, _compressedSize); - _storedUncompressedData = new MemoryStream(initialCapacity); + _storedUncompressedData = new MemoryStream((int)_uncompressedSize); Stream decompressed = BuildDecompressionPipeline(decryptedStream); diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs index 7d9864b5553280..4dd6b616f35bb6 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs @@ -596,23 +596,6 @@ private string DecodeEntryString(byte[] entryStringBytes) // will not work in a 32-bit process. private static readonly bool s_allowLargeZipArchiveEntriesInUpdateMode = IntPtr.Size > 4; - // A DEFLATE length/distance pair can encode a back-reference match of at most 258 bytes - // (RFC 1951), so a single, non-nested DEFLATE stream cannot legitimately expand by more - // than about 1032x (258 bytes of match per few bits of compressed data). This bounds how - // large a declared uncompressed size can plausibly be relative to the actual compressed - // data present in the archive. - private const long MaxDeflateExpansionRatio = 1032; - - // Deflate64 extends the maximum match length from 258 up to 65538 bytes (see - // InflaterManaged/OutputWindow), so its worst-case expansion ratio scales accordingly. - // 256x is a round, conservative multiplier that comfortably covers the ~254x increase - // in maximum match length without relying on an exact derivation. - private const long MaxDeflate64ExpansionRatio = MaxDeflateExpansionRatio * 256; - - // A small additive allowance so that tiny, legitimate entries (where fixed per-stream - // overhead dominates the ratio math) are never rejected. - private const long MinPlausibleUncompressedSizeAllowance = 4096; - internal bool EverOpenedForWrite => _everOpenedForWrite; internal long GetOffsetOfCompressedData() @@ -671,48 +654,6 @@ internal void ReadEncryptionSaltIfNeeded() } } - // Validates that this entry's declared _uncompressedSize is plausible given its _compressedSize, - // i.e. that it could actually have been produced by decompressing the bytes physically present - // in the archive. Without this check, an entry can declare an uncompressed size wildly out of - // proportion to its (small) compressed size, causing GetUncompressedData/GetUncompressedDataAsync - // to eagerly allocate a MemoryStream sized to that untrusted value before any decompression is - // attempted - e.g. a few hundred bytes of archive claiming multiple gigabytes of uncompressed - // content. Only applies to entries whose data originates from the archive being read; entries - // created fresh in this session have no untrusted header to validate. - private void ValidateUncompressedSizeIsPlausible() - { - if (!_originallyInArchive) - { - return; - } - - if (CompressionMethod == ZipCompressionMethod.Stored) - { - if (_uncompressedSize != _compressedSize) - { - _currentlyOpenForWrite = false; - throw new InvalidDataException(SR.Format(SR.EntryUncompressedSizeImplausible, _uncompressedSize, _compressedSize)); - } - - return; - } - - long maxExpansionRatio = CompressionMethod == ZipCompressionMethod.Deflate64 - ? MaxDeflate64ExpansionRatio - : MaxDeflateExpansionRatio; - - if (_uncompressedSize > MinPlausibleUncompressedSizeAllowance) - { - // Divide rather than multiply to avoid any risk of overflow. - long minPlausibleCompressedSize = (_uncompressedSize - MinPlausibleUncompressedSizeAllowance) / maxExpansionRatio; - if (_compressedSize < minPlausibleCompressedSize) - { - _currentlyOpenForWrite = false; - throw new InvalidDataException(SR.Format(SR.EntryUncompressedSizeImplausible, _uncompressedSize, _compressedSize)); - } - } - } - private MemoryStream GetUncompressedData(ReadOnlySpan password = default) { if (_storedUncompressedData == null) @@ -728,13 +669,7 @@ private MemoryStream GetUncompressedData(ReadOnlySpan password = default) throw new InvalidDataException(SR.EntryUncompressedSizeTooLargeForUpdateMode); } - ValidateUncompressedSizeIsPlausible(); - - // _compressedSize is already validated against the archive's real length (above), so - // capping capacity to it avoids a huge allocation from a small archive; the stream - // still grows normally if the entry decompresses larger than this. - int initialCapacity = (int)Math.Min(_uncompressedSize, _compressedSize); - _storedUncompressedData = new MemoryStream(initialCapacity); + _storedUncompressedData = new MemoryStream((int)_uncompressedSize); if (_originallyInArchive) { diff --git a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs index a3a6fb5b7ac8f1..43ce9d2f78b554 100644 --- a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs +++ b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs @@ -449,178 +449,6 @@ public static async Task ZipArchiveEntry_OpenInUpdateMode_UncompressedSizeGreate await DisposeZipArchive(async, archive); } - [Theory] - [MemberData(nameof(Get_Booleans_Data))] - public static async Task ZipArchiveEntry_OpenInUpdateMode_UncompressedSizeImplausibleGivenCompressedSize_ThrowsInvalidData(bool async) - { - // A small compressed entry cannot legitimately decompress to a wildly larger size: a single - // DEFLATE stream can expand by, at most, a few orders of magnitude (see MaxDeflateExpansionRatio). - // If the declared _uncompressedSize is implausible relative to the actual _compressedSize present - // in the archive (as could happen with a spoofed/corrupt header), the entry must be rejected up - // front in Update mode, before a MemoryStream sized to the untrusted declared value is allocated. - byte[] payload = [0xCA, 0xFE, 0xBA, 0xBE, 0xDE, 0xAD, 0xBE, 0xEF]; - MemoryStream stream = new MemoryStream(); - - // Use an actual Deflate-compressed entry (not Stored): the plausibility check only - // applies to compression methods that can expand data, and Stored cannot. - ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Create, leaveOpen: true); - ZipArchiveEntry entry = archive.CreateEntry("entry.bin", CompressionLevel.Optimal); - Stream entryStream = await OpenEntryStream(async, entry); - await entryStream.WriteAsync(payload); - await DisposeStream(async, entryStream); - await DisposeZipArchive(async, archive); - - stream.Position = 0; - archive = await CreateZipArchive(async, stream, ZipArchiveMode.Update, leaveOpen: true); - entry = archive.GetEntry("entry.bin"); - - // The entry's actual compressed size is a handful of bytes; claim a 50 MB uncompressed size, - // which is many orders of magnitude beyond what that compressed data could plausibly produce. - FieldInfo uncompressedSizeField = typeof(ZipArchiveEntry).GetField("_uncompressedSize", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(uncompressedSizeField); - uncompressedSizeField.SetValue(entry, 50_000_000L); - - if (async) - { - await Assert.ThrowsAsync(() => entry.OpenAsync()); - } - else - { - Assert.Throws(() => entry.Open()); - } - - await DisposeZipArchive(async, archive); - } - - [Theory] - [MemberData(nameof(Get_Booleans_Data))] - public static async Task ZipArchiveEntry_OpenInUpdateMode_HighButPlausibleCompressionRatio_OpensSuccessfully(bool async) - { - // Legitimately compressible content (a highly repetitive payload) can have a large, but - // plausible, expansion ratio. This must continue to open successfully in Update mode. - byte[] payload = new byte[1_000_000]; - Array.Fill(payload, (byte)'A'); - MemoryStream stream = new MemoryStream(); - - ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Create, leaveOpen: true); - ZipArchiveEntry entry = archive.CreateEntry("entry.bin", CompressionLevel.Optimal); - Stream entryStream = await OpenEntryStream(async, entry); - await entryStream.WriteAsync(payload); - await DisposeStream(async, entryStream); - await DisposeZipArchive(async, archive); - - stream.Position = 0; - archive = await CreateZipArchive(async, stream, ZipArchiveMode.Update, leaveOpen: true); - entry = archive.GetEntry("entry.bin"); - - using (MemoryStream ms = new MemoryStream()) - { - Stream source = await OpenEntryStream(async, entry); - await source.CopyToAsync(ms); - Assert.Equal(payload.Length, ms.Length); - await DisposeStream(async, source); - } - - await DisposeZipArchive(async, archive); - } - - [Theory] - [MemberData(nameof(Get_Booleans_Data))] - public static async Task ZipArchiveEntry_OpenInUpdateMode_LargePlausibleClaimedSize_DoesNotPreallocateFullClaimedSize(bool async) - { - // A mathematically-plausible but untrue _uncompressedSize claim should not cause the initial - // MemoryStream to be pre-allocated to that claimed size; it should be capped at - // _compressedSize instead, and grow normally only as real data is copied in. - // - // The payload is incompressible so _compressedSize stays close to its length, keeping - // IsOpenableFinalVerifications satisfied while we lie about _uncompressedSize below. - byte[] payload = new byte[900_000]; - new Random(42).NextBytes(payload); - MemoryStream stream = new MemoryStream(); - - ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Create, leaveOpen: true); - ZipArchiveEntry entry = archive.CreateEntry("entry.bin", CompressionLevel.Fastest); - Stream entryStream = await OpenEntryStream(async, entry); - await entryStream.WriteAsync(payload); - await DisposeStream(async, entryStream); - await DisposeZipArchive(async, archive); - - stream.Position = 0; - archive = await CreateZipArchive(async, stream, ZipArchiveMode.Update, leaveOpen: true); - entry = archive.GetEntry("entry.bin"); - - // The real _compressedSize (~900,000 bytes) makes a 500 MB claim mathematically plausible - // under ValidateUncompressedSizeIsPlausible, even though the real content is far smaller. - FieldInfo uncompressedSizeField = typeof(ZipArchiveEntry).GetField("_uncompressedSize", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(uncompressedSizeField); - uncompressedSizeField.SetValue(entry, 500_000_000L); - - FieldInfo compressedSizeField = typeof(ZipArchiveEntry).GetField("_compressedSize", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(compressedSizeField); - long realCompressedSize = Assert.IsType(compressedSizeField.GetValue(entry)); - - Stream source = await OpenEntryStream(async, entry); - - using (MemoryStream ms = new MemoryStream()) - { - await source.CopyToAsync(ms); - Assert.Equal(payload.Length, ms.Length); - Assert.Equal(payload, ms.ToArray()); - } - - await DisposeStream(async, source); - - FieldInfo storedUncompressedDataField = typeof(ZipArchiveEntry).GetField("_storedUncompressedData", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(storedUncompressedDataField); - MemoryStream storedUncompressedData = Assert.IsType(storedUncompressedDataField.GetValue(entry)); - - // The real capacity should track the physically-supplied compressed size, not the untrusted - // 500 MB claim. Bound this against realCompressedSize rather than asserting exact equality: - // exact equality would depend on Deflate never compressing this payload below its original - // length, which isn't a guaranteed property of the compressor. - Assert.True(storedUncompressedData.Capacity <= realCompressedSize * 2); - Assert.True(storedUncompressedData.Capacity < 500_000_000 / 10); - - await DisposeZipArchive(async, archive); - } - - [Theory] - [MemberData(nameof(Get_Booleans_Data))] - [SkipOnPlatform(TestPlatforms.Browser, "WinZip AES encryption is not supported on browser.")] - public static async Task ZipArchiveEntry_OpenWithPasswordInUpdateMode_UncompressedSizeImplausibleGivenCompressedSize_ThrowsInvalidData(bool async) - { - // The encrypted re-open-for-update path decrypts and decompresses into a MemoryStream sized to - // the declared uncompressed size. In the async implementation this happens via a separate code - // path (StoreDecryptedDataForUpdateAsync) from the unencrypted GetUncompressedDataAsync path, - // while the sync implementation shares GetUncompressedData(password) with the unencrypted path. - // Both must apply the same uncompressed-size plausibility check. - const string Password = "S3cur3P@ssw0rd"; - byte[] payload = [0xCA, 0xFE, 0xBA, 0xBE, 0xDE, 0xAD, 0xBE, 0xEF]; - MemoryStream stream = new MemoryStream(); - - ZipArchive createArchive = await CreateZipArchive(async, stream, ZipArchiveMode.Create, leaveOpen: true); - ZipArchiveEntry createEntry = createArchive.CreateEntry("entry.bin", CompressionLevel.Optimal, Password, ZipEncryptionMethod.Aes256); - Stream entryStream = await OpenEntryStream(async, createEntry, Password); - await entryStream.WriteAsync(payload); - await DisposeStream(async, entryStream); - await DisposeZipArchive(async, createArchive); - - stream.Position = 0; - ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Update, leaveOpen: true); - ZipArchiveEntry entry = archive.GetEntry("entry.bin"); - Assert.True(entry.IsEncrypted); - - // The entry's actual compressed size is a handful of bytes; claim a 50 MB uncompressed size, - // which is many orders of magnitude beyond what that compressed data could plausibly produce. - FieldInfo uncompressedSizeField = typeof(ZipArchiveEntry).GetField("_uncompressedSize", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(uncompressedSizeField); - uncompressedSizeField.SetValue(entry, 50_000_000L); - - await Assert.ThrowsAsync(() => OpenEntryStream(async, entry, Password)); - - await DisposeZipArchive(async, archive); - } - [Theory] [MemberData(nameof(Get_Booleans_Data))] public static async Task UnseekableVeryLargeArchive_DataDescriptor_Read_Zip64(bool async) diff --git a/src/libraries/System.IO.Packaging/src/Resources/Strings.resx b/src/libraries/System.IO.Packaging/src/Resources/Strings.resx index 536ebbe57483ae..bec9f0ceb4215c 100644 --- a/src/libraries/System.IO.Packaging/src/Resources/Strings.resx +++ b/src/libraries/System.IO.Packaging/src/Resources/Strings.resx @@ -72,6 +72,9 @@ ContentType string cannot have leading/trailing Linear White Spaces [LWS - RFC 2616]. + + The '[Content_Types].xml' part exceeds the maximum allowed size of {0} bytes. + Unrecognized root element in Core Properties part. diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs index f3e1a416e360b6..5d9015740b29ee 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs @@ -1141,6 +1141,11 @@ private void ParseContentTypesFile(System.Collections.ObjectModel.ReadOnlyCollec throw new FormatException(SR.BadPackageFormat); } + if (_contentTypeZipArchiveEntry.Length > MaxContentTypesXmlSize) + { + throw new FileFormatException(SR.Format(SR.ContentTypeStreamTooLarge, MaxContentTypesXmlSize)); + } + _contentTypeStreamExists = true; return _zipStreamManager.Open(_contentTypeZipArchiveEntry, FileAccess.ReadWrite); } @@ -1304,6 +1309,17 @@ private static void ThrowIfXmlAttributeMissing(string attributeName, string? att private CompressionLevel _cachedCompressionLevel; private const string ContentTypesFile = "[Content_Types].xml"; private const string ContentTypesFileUpperInvariant = "[CONTENT_TYPES].XML"; + + // Maximum allowed (uncompressed) size for the "[Content_Types].xml" part. This part is package + // metadata rather than user content, so - similarly to the metadata block size limit applied to + // TAR archives (see https://github.com/dotnet/runtime/pull/127602) - its size can be bounded to a + // value that comfortably covers legitimate packages while preventing a small, malformed, or + // malicious archive from declaring an implausibly large entry that gets eagerly buffered in memory + // when the package is opened for ReadWrite access. 4 MB covers packages with 100,000+ parts even in + // the worst case where every part has a distinct content type (forcing an element each), + // which is far beyond what real-world OPC packages (e.g. Office documents) contain. + private const long MaxContentTypesXmlSize = 4 * 1024 * 1024; + private const int DefaultDictionaryInitialSize = 16; private const int OverrideDictionaryInitialSize = 8; diff --git a/src/libraries/System.IO.Packaging/tests/Tests.cs b/src/libraries/System.IO.Packaging/tests/Tests.cs index fa54a7cdc2f3ea..d7be1f72be2e1a 100644 --- a/src/libraries/System.IO.Packaging/tests/Tests.cs +++ b/src/libraries/System.IO.Packaging/tests/Tests.cs @@ -103,22 +103,18 @@ public void GetStreamCreate_OverwritesExistingPartContentWithoutLeftoverBytes(Fi } [Fact] - [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Desktop's built-in System.IO.Packaging implementation wraps the size-mismatch error in a FileFormatException instead of throwing InvalidDataException directly")] - public void Open_ContentTypesEntryWithImplausibleDeclaredUncompressedSize_ThrowsInvalidDataException() + public void Open_ContentTypesEntryDeclaredSizeExceedsMaximum_ThrowsFileFormatException() { // Regression test: Package.Open's default ReadWrite access automatically parses the mandatory - // [Content_Types].xml part during Open(). If that entry's declared (but untrusted) uncompressed - // size is implausible relative to its actual compressed size - as would happen with a corrupt - // or maliciously crafted header - Open() must reject the archive instead of eagerly allocating - // a buffer sized to the attacker-controlled value. + // [Content_Types].xml part during Open(). This part is package metadata, not user content, so + // its declared (but untrusted) uncompressed size must be bounded to a sane maximum. Otherwise a + // small, corrupt, or maliciously crafted archive could declare an implausibly large entry that + // gets eagerly buffered in memory (as a MemoryStream sized to the declared value) when the + // package is opened for ReadWrite access. FileInfo file = GetTempFileInfoWithExtension(".zip"); using (Package package = Package.Open(file.FullName, FileMode.Create, FileAccess.ReadWrite)) { - // The [Content_Types].xml entry inherits its compression level from the first part added - // to the package, so a compressed option (rather than the default NotCompressed/Stored) is - // required here for the entry to be Deflate-compressed and therefore subject to the - // uncompressed-size plausibility check under test. PackagePart part = package.CreatePart( PackUriHelper.CreatePartUri(new Uri("MyFile.xml", UriKind.Relative)), Mime_MediaTypeNames_Text_Xml, @@ -129,15 +125,17 @@ public void Open_ContentTypesEntryWithImplausibleDeclaredUncompressedSize_Throws } byte[] archiveBytes = File.ReadAllBytes(file.FullName); - PatchContentTypesUncompressedSize(archiveBytes, implausibleUncompressedSize: 500_000_000); + // Comfortably above the 4 MB cap, but small enough that a regression in the guard would not + // risk a large allocation while running this test. + PatchContentTypesUncompressedSize(archiveBytes, oversizedUncompressedSize: 5_000_000); File.WriteAllBytes(file.FullName, archiveBytes); - Assert.Throws(() => Package.Open(file.FullName, FileMode.Open, FileAccess.ReadWrite)); + Assert.Throws(() => Package.Open(file.FullName, FileMode.Open, FileAccess.ReadWrite)); } // Patches the declared uncompressed size field (in both the local file header and the central // directory record) for the "[Content_Types].xml" entry within a raw, in-memory zip byte array. - private static void PatchContentTypesUncompressedSize(byte[] archiveBytes, uint implausibleUncompressedSize) + private static void PatchContentTypesUncompressedSize(byte[] archiveBytes, uint oversizedUncompressedSize) { const string EntryName = "[Content_Types].xml"; byte[] nameBytes = Encoding.ASCII.GetBytes(EntryName); @@ -161,14 +159,14 @@ private static void PatchContentTypesUncompressedSize(byte[] archiveBytes, uint // uncompressed size field is the 4 bytes located 8 bytes before the file name starts. if (nameIndex >= 30 && archiveSpan.Slice(nameIndex - 30, 4).SequenceEqual(localHeaderSignature)) { - BinaryPrimitives.WriteUInt32LittleEndian(archiveSpan.Slice(nameIndex - 8, 4), implausibleUncompressedSize); + BinaryPrimitives.WriteUInt32LittleEndian(archiveSpan.Slice(nameIndex - 8, 4), oversizedUncompressedSize); patchedCount++; } // Central directory file header: fixed 46-byte header immediately precedes the file name; // the uncompressed size field is the 4 bytes located 22 bytes before the file name starts. else if (nameIndex >= 46 && archiveSpan.Slice(nameIndex - 46, 4).SequenceEqual(centralDirectorySignature)) { - BinaryPrimitives.WriteUInt32LittleEndian(archiveSpan.Slice(nameIndex - 22, 4), implausibleUncompressedSize); + BinaryPrimitives.WriteUInt32LittleEndian(archiveSpan.Slice(nameIndex - 22, 4), oversizedUncompressedSize); patchedCount++; } } From ea1c9bc593c39d54f495a315ef39b77fd0642a9e Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Wed, 9 Sep 2026 14:32:49 +0200 Subject: [PATCH 7/9] check total size of pieces too --- .../src/System/IO/Packaging/ZipPackage.cs | 11 +++++++++++ .../System.IO.Packaging/tests/PartPieceTests.cs | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs index 5d9015740b29ee..9b7aa0bed35ec5 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs @@ -1152,6 +1152,17 @@ private void ParseContentTypesFile(System.Collections.ObjectModel.ReadOnlyCollec // If the content type stream is interleaved, validate the piece numbering. else if (partPieces != null) { + long totalLength = 0; + foreach (ZipPackagePartPiece piece in partPieces) + { + totalLength += piece.ZipArchiveEntry.Length; + } + + if (totalLength > MaxContentTypesXmlSize) + { + throw new FileFormatException(SR.Format(SR.ContentTypeStreamTooLarge, MaxContentTypesXmlSize)); + } + _contentTypeStreamExists = true; _contentTypeStreamPieces = partPieces; diff --git a/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs b/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs index 37736f4d6d7f6f..e3cd9725f1ae00 100644 --- a/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs +++ b/src/libraries/System.IO.Packaging/tests/PartPieceTests.cs @@ -271,6 +271,23 @@ public void CanParseInterleavedContentTypesFile() Assert.NotEmpty(zipPackage.GetParts()); } + // Regression test: an interleaved "[Content_Types].xml" (i.e. one split into pieces) must be + // bounded by the same maximum size as an atomic "[Content_Types].xml", since its pieces are + // recombined and parsed by the same XmlReader. Otherwise a malicious package could bypass the + // atomic-entry size guard simply by splitting the content types part into pieces. + [Fact] + public void InterleavedContentTypesExceedingMaxSizeThrows() + { + // Two highly-compressible (all-zero) pieces whose combined declared uncompressed size + // exceeds the 4 MB cap, even though neither piece alone does. + byte[] package = CreatePackage( + new PartConstructionParameters("AtomicPartEntry.bin", true, false, false, false, [200], GenerateRandomBytes), + new PartConstructionParameters("[Content_Types].xml", false, true, false, false, [2_500_000, 2_500_001], (_, totalLength) => new byte[totalLength])); + + using var ms = new MemoryStream(package); + Assert.Throws(() => Package.Open(ms)); + } + // Verify that the IComparable implementation on ZipPackagePartPiece works properly. // If it is, we should see the list reordered by piece number [Theory] From 096969e379801acb9fcc9b6e1a91ff47ce7f9c6d Mon Sep 17 00:00:00 2001 From: alinpahontu2912 Date: Thu, 10 Sep 2026 14:50:29 +0200 Subject: [PATCH 8/9] add overflow logic for summing up parts' content length --- .../src/System/IO/Packaging/ZipPackage.cs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs index 9b7aa0bed35ec5..368cda52615867 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs @@ -1152,15 +1152,21 @@ private void ParseContentTypesFile(System.Collections.ObjectModel.ReadOnlyCollec // If the content type stream is interleaved, validate the piece numbering. else if (partPieces != null) { + // Sum the piece lengths without risking overflow: each ZipArchiveEntry.Length is a + // non-negative long that (with Zip64) can be as large as long.MaxValue, so a malicious + // archive could otherwise make an unchecked running total wrap around and defeat the + // size check below. Comparing against the remaining budget instead of adding first keeps + // totalLength bounded by MaxContentTypesXmlSize at all times. long totalLength = 0; foreach (ZipPackagePartPiece piece in partPieces) { - totalLength += piece.ZipArchiveEntry.Length; - } + long pieceLength = piece.ZipArchiveEntry.Length; + if (pieceLength > MaxContentTypesXmlSize - totalLength) + { + throw new FileFormatException(SR.Format(SR.ContentTypeStreamTooLarge, MaxContentTypesXmlSize)); + } - if (totalLength > MaxContentTypesXmlSize) - { - throw new FileFormatException(SR.Format(SR.ContentTypeStreamTooLarge, MaxContentTypesXmlSize)); + totalLength += pieceLength; } _contentTypeStreamExists = true; From 353f0cf5074aea37e619d8e7bc3f7c2e3102df61 Mon Sep 17 00:00:00 2001 From: Stefan-Alin Pahontu <56953855+alinpahontu2912@users.noreply.github.com> Date: Thu, 10 Sep 2026 15:35:06 +0200 Subject: [PATCH 9/9] Change file access mode to read-only for zip stream Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs index 368cda52615867..9f77bee64dd6e2 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/ZipPackage.cs @@ -1147,7 +1147,7 @@ private void ParseContentTypesFile(System.Collections.ObjectModel.ReadOnlyCollec } _contentTypeStreamExists = true; - return _zipStreamManager.Open(_contentTypeZipArchiveEntry, FileAccess.ReadWrite); + return _zipStreamManager.Open(_contentTypeZipArchiveEntry, FileAccess.Read); } // If the content type stream is interleaved, validate the piece numbering. else if (partPieces != null)