diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs index ee5bd67e21f65c..9e6c5ba913efae 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs @@ -257,9 +257,18 @@ private static bool TryGetZip64BlockFromGenericExtraField(ZipGenericExtraField e // 2. When the size indicates that all the information is available ("slightly invalid files"). bool readAllFields = extraField.Size >= MaximumExtraFieldLength; + // The original values are unsigned 64-bit, so a negative signed value means the + // value does not fit in Int64 and cannot be represented by the rest of the API + // (which uses long). Validate each field as it is read so that short extra fields + // (which exit early below) cannot bypass the check. + if (readUncompressedSize) { zip64Block._uncompressedSize = BinaryPrimitives.ReadInt64LittleEndian(data); + if (zip64Block._uncompressedSize < 0) + { + throw new InvalidDataException(SR.FieldTooBigUncompressedSize); + } data = data.Slice(FieldLengths.UncompressedSize); } else if (readAllFields) @@ -275,6 +284,10 @@ private static bool TryGetZip64BlockFromGenericExtraField(ZipGenericExtraField e if (readCompressedSize) { zip64Block._compressedSize = BinaryPrimitives.ReadInt64LittleEndian(data); + if (zip64Block._compressedSize < 0) + { + throw new InvalidDataException(SR.FieldTooBigCompressedSize); + } data = data.Slice(FieldLengths.CompressedSize); } else if (readAllFields) @@ -290,6 +303,10 @@ private static bool TryGetZip64BlockFromGenericExtraField(ZipGenericExtraField e if (readLocalHeaderOffset) { zip64Block._localHeaderOffset = BinaryPrimitives.ReadInt64LittleEndian(data); + if (zip64Block._localHeaderOffset < 0) + { + throw new InvalidDataException(SR.FieldTooBigLocalHeaderOffset); + } data = data.Slice(FieldLengths.LocalHeaderOffset); } else if (readAllFields) @@ -307,20 +324,6 @@ private static bool TryGetZip64BlockFromGenericExtraField(ZipGenericExtraField e zip64Block._startDiskNumber = BinaryPrimitives.ReadUInt32LittleEndian(data); } - // original values are unsigned, so implies value is too big to fit in signed integer - if (zip64Block._uncompressedSize < 0) - { - throw new InvalidDataException(SR.FieldTooBigUncompressedSize); - } - if (zip64Block._compressedSize < 0) - { - throw new InvalidDataException(SR.FieldTooBigCompressedSize); - } - if (zip64Block._localHeaderOffset < 0) - { - throw new InvalidDataException(SR.FieldTooBigLocalHeaderOffset); - } - return true; } 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 46d899426459e8..fa8abacb90ed3f 100644 --- a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs +++ b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs @@ -2434,5 +2434,127 @@ public static async Task OpenWithFileAccess_DisposedArchive_Throws(bool async) Assert.Throws(() => entry.Open(FileAccess.Read)); await Assert.ThrowsAsync(() => entry.OpenAsync(FileAccess.Read)); } + + public enum NegativeZip64Field { UncompressedSize, CompressedSize, LocalHeaderOffset } + + public static IEnumerable Zip64ExtraField_NegativeField_Data() => + from async in new[] { true, false } + from field in new[] { NegativeZip64Field.UncompressedSize, NegativeZip64Field.CompressedSize, NegativeZip64Field.LocalHeaderOffset } + select new object[] { async, field }; + + [Theory] + [MemberData(nameof(Zip64ExtraField_NegativeField_Data))] + public static async Task Zip64ExtraField_NegativeField_Throws(bool async, NegativeZip64Field negativeField) + { + // A ZIP64 extra that encodes a 64-bit size/offset as 0xFFFF_FFFF_FFFF_FFFF (read as -1L) + // is malformed: the long-based public surface cannot represent it. The central directory + // parser must reject it before callers observe a negative Length / CompressedLength / + // local header offset on a ZipArchiveEntry. + // + // CreateAsync reads the central directory eagerly; the sync ctor defers until .Entries + // is accessed, so we force that read below. + byte[] zipArchive = CreateZipWithNegativeZip64Field(negativeField); + + await Assert.ThrowsAsync(async () => + { + ZipArchive? archive = null; + try + { + archive = await CreateZipArchive(async, new MemoryStream(zipArchive), ZipArchiveMode.Read); + _ = archive.Entries; + } + finally + { + if (archive is not null) + { + await DisposeZipArchive(async, archive); + } + } + }); + } + + private static byte[] CreateZipWithNegativeZip64Field(NegativeZip64Field negativeField) + { + // Minimal ZIP whose 32-bit size fields (and, for the local-header-offset case, the + // 32-bit relative-offset field) use the ZIP64 sentinel 0xFFFFFFFF so the parser is + // forced to read from the ZIP64 extra. Exactly one slot in the extra holds -1L, + // targeting the matching FieldTooBig* throw in Zip64ExtraField.TryGetZip64Block... + const uint Sentinel32 = 0xFFFFFFFF; + const ushort Zip64Tag = 1; + + bool includeOffset = negativeField == NegativeZip64Field.LocalHeaderOffset; + ushort zip64DataSize = (ushort)(includeOffset ? 24 : 16); + ushort zip64TotalSize = (ushort)(4 + zip64DataSize); + + long uncompressed = negativeField == NegativeZip64Field.UncompressedSize ? -1L : 0L; + long compressed = negativeField == NegativeZip64Field.CompressedSize ? -1L : 0L; + long offset = includeOffset ? -1L : 0L; + uint relativeOffsetSmall = includeOffset ? Sentinel32 : 0u; + + byte[] name = Encoding.UTF8.GetBytes("test.txt"); + using MemoryStream ms = new(); + using BinaryWriter w = new(ms, Encoding.UTF8, leaveOpen: true); + + void WriteZip64Extra() + { + w.Write(Zip64Tag); + w.Write(zip64DataSize); + w.Write(uncompressed); + w.Write(compressed); + if (includeOffset) + { + w.Write(offset); + } + } + + // Local file header + w.Write(0x04034b50u); // signature + w.Write((ushort)45); // version needed + w.Write((ushort)0); // gp flags + w.Write((ushort)0); // method + w.Write(0u); // mod time/date + w.Write(0u); // crc32 + w.Write(Sentinel32); // compressed size + w.Write(Sentinel32); // uncompressed size + w.Write((ushort)name.Length); + w.Write(zip64TotalSize); + w.Write(name); + WriteZip64Extra(); + + long centralDirOffset = ms.Position; + + // Central directory header + w.Write(0x02014b50u); // signature + w.Write((ushort)45); // version made by + w.Write((ushort)45); // version needed + w.Write((ushort)0); // gp flags + w.Write((ushort)0); // method + w.Write(0u); // mod time/date + w.Write(0u); // crc32 + w.Write(Sentinel32); // compressed size + w.Write(Sentinel32); // uncompressed size + w.Write((ushort)name.Length); + w.Write(zip64TotalSize); + w.Write((ushort)0); // file comment length + w.Write((ushort)0); // disk number start + w.Write((ushort)0); // internal attrs + w.Write(0u); // external attrs + w.Write(relativeOffsetSmall); // relative offset of local header + w.Write(name); + WriteZip64Extra(); + + long centralDirSize = ms.Position - centralDirOffset; + + // End of central directory + w.Write(0x06054b50u); // signature + w.Write(0u); // disk numbers + w.Write((ushort)1); // entries on disk + w.Write((ushort)1); // total entries + w.Write((uint)centralDirSize); + w.Write((uint)centralDirOffset); + w.Write((ushort)0); // comment length + + return ms.ToArray(); + } } }