Fix reading Zip64 end of central directory locator (#118239)

* Fix reading Zip64 end of central directory locator

* Update src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.Async.cs

Co-authored-by: Copilot <[email protected]>

* Fix build

* Add test

* Update src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs

* Handle another offset out of bounds case.

* Improve ZipArchiveFuzzer

* Add another test case

* Add a comment

---------

Co-authored-by: Copilot <[email protected]>

Author: rzikm

src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/ZipArchiveFuzzer.cs

@@ -15,56 +15,73 @@ internal sealed class ZipArchiveFuzzer : IFuzzer
 
     public void FuzzTarget(ReadOnlySpan<byte> bytes)
     {
-
         if (bytes.IsEmpty)
         {
             return;
         }
 
-        try
-        {
-            using var stream = new MemoryStream(bytes.ToArray());
-
-            Task sync_test = TestArchive(stream, async: false);
-            Task async_test = TestArchive(stream, async: true);
-
-            Task.WaitAll(sync_test, async_test);
-        }
-        catch (Exception) { }
+        TestArchive(CopyToRentedArray(bytes), bytes.Length, async: false).GetAwaiter().GetResult();
+        TestArchive(CopyToRentedArray(bytes), bytes.Length, async: true).GetAwaiter().GetResult();
     }
 
-    private async Task TestArchive(Stream stream, bool async)
+    private byte[] CopyToRentedArray(ReadOnlySpan<byte> bytes)
     {
-        stream.Position = 0;
-
-        ZipArchive archive;
-
-        if (async)
+        byte[] buffer = ArrayPool<byte>.Shared.Rent(bytes.Length);
+        try
         {
-            archive = await ZipArchive.CreateAsync(stream, ZipArchiveMode.Read, leaveOpen: false, entryNameEncoding: null);
+            bytes.CopyTo(buffer);
+            return buffer;
         }
-        else
+        catch
         {
-            archive = new ZipArchive(stream, ZipArchiveMode.Read, leaveOpen: false, entryNameEncoding: null);
+            ArrayPool<byte>.Shared.Return(buffer);
+            throw;
         }
+    }
 
-        foreach (var entry in archive.Entries)
+    private async Task TestArchive(byte[] buffer, int length, bool async)
+    {
+        try
         {
-            // Access entry properties to simulate usage
-            _ = entry.FullName;
-            _ = entry.Length;
-            _ = entry.Comment;
-            _ = entry.LastWriteTime;
-            _ = entry.Crc32;
-        }
+            using var stream = new MemoryStream(buffer, 0, length);
 
-        if (async)
+            ZipArchive archive;
+
+            if (async)
+            {
+                archive = await ZipArchive.CreateAsync(stream, ZipArchiveMode.Read, leaveOpen: false, entryNameEncoding: null);
+            }
+            else
+            {
+                archive = new ZipArchive(stream, ZipArchiveMode.Read, leaveOpen: false, entryNameEncoding: null);
+            }
+
+            foreach (var entry in archive.Entries)
+            {
+                // Access entry properties to simulate usage
+                _ = entry.FullName;
+                _ = entry.Length;
+                _ = entry.Comment;
+                _ = entry.LastWriteTime;
+                _ = entry.Crc32;
+            }
+
+            if (async)
+            {
+                await archive.DisposeAsync();
+            }
+            else
+            {
+                archive.Dispose();
+            }
+        }
+        catch (InvalidDataException)
         {
-            await archive.DisposeAsync();
+            // ignore, this exception is expected to be thrown for invalid/corrupted archives.
         }
-        else
+        finally
         {
-            archive.Dispose();
+            ArrayPool<byte>.Shared.Return(buffer);
         }
     }
 }

src/libraries/System.IO.Compression/src/Resources/Strings.resx

@@ -293,4 +293,7 @@
   <data name="PlatformNotSupported_Compression" xml:space="preserve">
     <value>System.IO.Compression is not supported on this platform.</value>
   </data>
+  <data name="InvalidOffsetToZip64EOCD" xml:space="preserve">
+    <value>Invalid offset to the Zip64 End of Central Directory record.</value>
+  </data>
 </root>

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.Async.cs

@@ -247,10 +247,12 @@ private async Task ReadEndOfCentralDirectoryAsync(CancellationToken cancellation
                     cancellationToken).ConfigureAwait(false))
                 throw new InvalidDataException(SR.EOCDNotFound);
 
+            long eocdStart = _archiveStream.Position;
+
             // read the EOCD
             ZipEndOfCentralDirectoryBlock eocd = await ZipEndOfCentralDirectoryBlock.ReadBlockAsync(_archiveStream, cancellationToken).ConfigureAwait(false);
 
-            ReadEndOfCentralDirectoryInnerWork(eocd, out long eocdStart);
+            ReadEndOfCentralDirectoryInnerWork(eocd);
 
             await TryReadZip64EndOfCentralDirectoryAsync(eocd, eocdStart, cancellationToken).ConfigureAwait(false);
 
@@ -284,6 +286,12 @@ private async ValueTask TryReadZip64EndOfCentralDirectoryAsync(ZipEndOfCentralDi
         {
             // Read Zip64 End of Central Directory Locator
 
+            // Check if there's enough space before the EOCD to look for the Zip64 EOCDL
+            if (eocdStart < Zip64EndOfCentralDirectoryLocator.TotalSize)
+            {
+                throw new InvalidDataException(SR.Zip64EOCDNotWhereExpected);
+            }
+
             // This seeks forwards almost to the beginning of the Zip64-EOCDL, one byte after where the signature would be located
             _archiveStream.Seek(eocdStart - Zip64EndOfCentralDirectoryLocator.SizeOfBlockWithoutSignature, SeekOrigin.Begin);
 

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs

@@ -587,10 +587,8 @@ private void ReadCentralDirectory()
             }
         }
 
-        private void ReadEndOfCentralDirectoryInnerWork(ZipEndOfCentralDirectoryBlock eocd, out long eocdStart)
+        private void ReadEndOfCentralDirectoryInnerWork(ZipEndOfCentralDirectoryBlock eocd)
         {
-            eocdStart = _archiveStream.Position;
-
             if (eocd.NumberOfThisDisk != eocd.NumberOfTheDiskWithTheStartOfTheCentralDirectory)
                 throw new InvalidDataException(SR.SplitSpanned);
 
@@ -624,10 +622,12 @@ private void ReadEndOfCentralDirectory()
                         ZipEndOfCentralDirectoryBlock.ZipFileCommentMaxLength + ZipEndOfCentralDirectoryBlock.FieldLengths.Signature))
                     throw new InvalidDataException(SR.EOCDNotFound);
 
+                long eocdStart = _archiveStream.Position;
+
                 // read the EOCD
                 ZipEndOfCentralDirectoryBlock eocd = ZipEndOfCentralDirectoryBlock.ReadBlock(_archiveStream);
 
-                ReadEndOfCentralDirectoryInnerWork(eocd, out long eocdStart);
+                ReadEndOfCentralDirectoryInnerWork(eocd);
 
                 TryReadZip64EndOfCentralDirectory(eocd, eocdStart);
 
@@ -653,6 +653,9 @@ private void TryReadZip64EndOfCentralDirectoryInnerInitialWork(Zip64EndOfCentral
 
             long zip64EOCDOffset = (long)locator.OffsetOfZip64EOCD;
 
+            if (zip64EOCDOffset < 0 || zip64EOCDOffset > _archiveStream.Length)
+                throw new InvalidDataException(SR.InvalidOffsetToZip64EOCD);
+
             _archiveStream.Seek(zip64EOCDOffset, SeekOrigin.Begin);
         }
 
@@ -686,6 +689,12 @@ private void TryReadZip64EndOfCentralDirectory(ZipEndOfCentralDirectoryBlock eoc
             {
                 // Read Zip64 End of Central Directory Locator
 
+                // Check if there's enough space before the EOCD to look for the Zip64 EOCDL
+                if (eocdStart < Zip64EndOfCentralDirectoryLocator.TotalSize)
+                {
+                    throw new InvalidDataException(SR.Zip64EOCDNotWhereExpected);
+                }
+
                 // This seeks forwards almost to the beginning of the Zip64-EOCDL, one byte after where the signature would be located
                 _archiveStream.Seek(eocdStart - Zip64EndOfCentralDirectoryLocator.SizeOfBlockWithoutSignature, SeekOrigin.Begin);
 
@@ -701,7 +710,6 @@ private void TryReadZip64EndOfCentralDirectory(ZipEndOfCentralDirectoryBlock eoc
 
                     // Read Zip64 End of Central Directory Record
                     Zip64EndOfCentralDirectoryRecord record = Zip64EndOfCentralDirectoryRecord.TryReadBlock(_archiveStream);
-
                     TryReadZip64EndOfCentralDirectoryInnerFinalWork(record);
                 }
             }

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.Async.cs

@@ -53,11 +53,18 @@ internal static async Task<bool> SeekBackwardsToSignatureAsync(Stream stream, Re
             bool signatureFound = false;
 
             int totalBytesRead = 0;
-            int duplicateBytesRead = 0;
 
-            while (!signatureFound && !outOfBytes && totalBytesRead <= maxBytesToRead)
+            while (!signatureFound && !outOfBytes && totalBytesRead < maxBytesToRead)
             {
-                int bytesRead = await SeekBackwardsAndReadAsync(stream, bufferMemory, signatureToFind.Length, cancellationToken).ConfigureAwait(false);
+                int overlap = totalBytesRead == 0 ? 0 : signatureToFind.Length;
+
+                if (maxBytesToRead - totalBytesRead + overlap < bufferMemory.Length)
+                {
+                    // If we have less than a full buffer left to read, we adjust the buffer size.
+                    bufferMemory = bufferMemory.Slice(0, maxBytesToRead - totalBytesRead + overlap);
+                }
+
+                int bytesRead = await SeekBackwardsAndReadAsync(stream, bufferMemory, overlap, cancellationToken).ConfigureAwait(false);
 
                 outOfBytes = bytesRead < bufferMemory.Length;
                 if (bytesRead < bufferMemory.Length)
@@ -68,15 +75,13 @@ internal static async Task<bool> SeekBackwardsToSignatureAsync(Stream stream, Re
                 bufferPointer = bufferMemory.Span.LastIndexOf(signatureToFind.Span);
                 Debug.Assert(bufferPointer < bufferMemory.Length);
 
-                totalBytesRead += (bufferMemory.Length - duplicateBytesRead);
+                totalBytesRead += bytesRead - overlap;
 
                 if (bufferPointer != -1)
                 {
                     signatureFound = true;
                     break;
                 }
-
-                duplicateBytesRead = signatureToFind.Length;
             }
 
             if (!signatureFound)

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.cs

@@ -124,11 +124,18 @@ internal static bool SeekBackwardsToSignature(Stream stream, ReadOnlySpan<byte>
             bool signatureFound = false;
 
             int totalBytesRead = 0;
-            int duplicateBytesRead = 0;
 
-            while (!signatureFound && !outOfBytes && totalBytesRead <= maxBytesToRead)
+            while (!signatureFound && !outOfBytes && totalBytesRead < maxBytesToRead)
             {
-                int bytesRead = SeekBackwardsAndRead(stream, bufferSpan, signatureToFind.Length);
+                int overlap = totalBytesRead == 0 ? 0 : signatureToFind.Length;
+
+                if (maxBytesToRead - totalBytesRead + overlap < bufferSpan.Length)
+                {
+                    // If we have less than a full buffer left to read, we adjust the buffer size.
+                    bufferSpan = bufferSpan.Slice(0, maxBytesToRead - totalBytesRead + overlap);
+                }
+
+                int bytesRead = SeekBackwardsAndRead(stream, bufferSpan, overlap);
 
                 outOfBytes = bytesRead < bufferSpan.Length;
                 if (bytesRead < bufferSpan.Length)
@@ -139,15 +146,13 @@ internal static bool SeekBackwardsToSignature(Stream stream, ReadOnlySpan<byte>
                 bufferPointer = bufferSpan.LastIndexOf(signatureToFind);
                 Debug.Assert(bufferPointer < bufferSpan.Length);
 
-                totalBytesRead += (bufferSpan.Length - duplicateBytesRead);
+                totalBytesRead += bytesRead - overlap;
 
                 if (bufferPointer != -1)
                 {
                     signatureFound = true;
                     break;
                 }
-
-                duplicateBytesRead = signatureToFind.Length;
             }
 
             if (!signatureFound)

src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs

@@ -195,7 +195,7 @@ public static async Task LargeArchive_DataDescriptor_Read_NonZip64_FileLengthGre
             Stream source = await OpenEntryStream(async, e);
             byte[] buffer = new byte[s_bufferSize];
             int read = await source.ReadAsync(buffer, 0, buffer.Length);   // We don't want to inflate this large archive entirely
-                                                                // just making sure it read successfully
+                                                                           // just making sure it read successfully
             Assert.Equal(s_bufferSize, read);
             foreach (byte b in buffer)
             {
@@ -564,7 +564,7 @@ public static async Task UpdateZipArchive_AddFileTo_ZipWithCorruptedFile(bool as
             byte[] buffer2 = new byte[1024];
             file.Seek(0, SeekOrigin.Begin);
 
-            while (await s.ReadAsync(buffer1, 0, buffer1.Length) != 0 )
+            while (await s.ReadAsync(buffer1, 0, buffer1.Length) != 0)
             {
                 await file.ReadAsync(buffer2, 0, buffer2.Length);
                 Assert.Equal(buffer1, buffer2);
@@ -1418,7 +1418,7 @@ public async Task ZipArchive_InvalidExtraFieldData_Async(byte validVersionToExtr
             // for first.bin would normally be skipped (because it hasn't changed) but it needs to be rewritten
             // because the central directory headers will be rewritten with a valid value and the local file header
             // needs to match.
-            await using (ZipArchive updatedArchive = await ZipArchive.CreateAsync(updatedStream, ZipArchiveMode.Update, leaveOpen: true, entryNameEncoding:  null))
+            await using (ZipArchive updatedArchive = await ZipArchive.CreateAsync(updatedStream, ZipArchiveMode.Update, leaveOpen: true, entryNameEncoding: null))
             {
                 ZipArchiveEntry newEntry = updatedArchive.CreateEntry("second.bin", CompressionLevel.NoCompression);
 
@@ -1659,6 +1659,39 @@ public static async Task NoSyncCallsWhenUsingAsync()
             }
         }
 
+        [Theory]
+        [MemberData(nameof(Get_Booleans_Data))]
+        public async Task ReadArchive_FrontTruncatedFile_Throws(bool async)
+        {
+            for (int i = 1; i < s_slightlyIncorrectZip64.Length - 1; i++)
+            {
+                await Assert.ThrowsAsync<InvalidDataException>(
+                    // The archive is truncated, so it should throw an exception
+                    () => CreateZipArchive(async, new MemoryStream(s_slightlyIncorrectZip64, i, s_slightlyIncorrectZip64.Length - i), ZipArchiveMode.Read));
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(Get_Booleans_Data))]
+        public async Task ReadArchive_Zip64EocdLocatorInvalidOffset_Throws(bool async)
+        {
+            byte[] data = s_slightlyIncorrectZip64.ToArray();
+
+            foreach (long offset in new[] { -1, int.MaxValue - 1 })
+            {
+                // Find Zip64 EOCD locator record
+                int eocdlOffset = data.AsSpan().LastIndexOf(new byte[] { 0x50, 0x4b, 0x06, 0x07 });
+                Assert.True(eocdlOffset >= 0, "Zip64 EOCD locator not found in test data");
+
+                // skip 4B signature and 4B index of disk, then overwrite the 8B offset to start of central directory
+                BinaryPrimitives.WriteInt64LittleEndian(data.AsSpan(eocdlOffset + 8, 8), offset);
+
+                await Assert.ThrowsAsync<InvalidDataException>(
+                    // The archive is truncated, so it should throw an exception
+                    () => CreateZipArchive(async, new MemoryStream(data), ZipArchiveMode.Read));
+            }
+        }
+
         // Generates a copy of s_invalidExtraFieldData with a variable number of bytes as extra field data.
         private static byte[] GenerateInvalidExtraFieldData(byte modifiedVersionToExtract, ushort extraFieldDataLength,
             out int lhVersionToExtractOffset,