Remove local header compressed/uncomressed size validation vs central directory records

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

@@ -749,7 +749,7 @@ private bool IsOpenable(bool needToUncompress, bool needToLoadIntoMemory, out st
                 }
                 Debug.Assert(_archive.ArchiveReader != null);
                 _archive.ArchiveStream.Seek(_offsetOfLocalHeader, SeekOrigin.Begin);
-                if (needToUncompress && !needToLoadIntoMemory)
+                if (needToUncompress && !needToLoadIntoMemory && ZipHelper.s_validateHeader)
                 {
                     if (!ZipLocalFileHeader.TryValidateBlock(_archive.ArchiveReader, this))
                     {

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

@@ -482,44 +482,91 @@ public static bool TryValidateBlock(BinaryReader reader, ZipArchiveEntry entry)
             }
             else
             {
-                if (reader.BaseStream.Length < reader.BaseStream.Position + extraFieldLength + entry.CompressedLength + 4)
+                // Data descriptor section can be written in 4 distinct scenarios
+                //   1. Data descriptor signature is optional, if signature is not set and the size info written in 4 bytes:
+                //                  crc-32                          4 bytes
+                //                  compressed size                 4 bytes
+                //                  uncompressed size               4 bytes
+                //
+                //   2. Data descriptor signature is set and size info written in 4 bytes
+                //                  data descriptor signature        4 bytes  (0x08074b50)
+                //                  crc-32                           4 bytes
+                //                  compressed size                  4 bytes
+                //                  uncompressed size                4 bytes
+                //
+                //   3. Data descriptor signature is not set and size info written in 8 bytes (Zip64)
+                //                  crc-32                           4 bytes
+                //                  compressed size                  8 bytes
+                //                  uncompressed size                8 bytes
+                //
+                //   4. Data descriptor signature is set and size info written in 8 bytes
+                //                  data descriptor signature        4 bytes  (0x08074b50)
+                //                  crc-32                           4 bytes
+                //                  compressed size                  8 bytes
+                //                  uncompressed size                8 bytes
+                //
+                // If there is enough bytes available for each data descriptor size
+                // read the bytes required to store that structure, and compare results
+
+                int minDataDescriptorSize = 12;
+                if (reader.BaseStream.Length < reader.BaseStream.Position + extraFieldLength + entry.CompressedLength + minDataDescriptorSize)
                 {
                     return false;
                 }
 
-                reader.BaseStream.Seek(extraFieldLength + entry.CompressedLength, SeekOrigin.Current); // seek to end of compressed file from which Data descriptor starts
-                uint dataDescriptorSignature = reader.ReadUInt32();
-                bool wasDataDescriptorSignatureRead = false;
-                int seekSize = 0;
-                if (dataDescriptorSignature == DataDescriptorSignature)
+                // Seek to end of compressed file from which Data descriptor section starts
+                reader.BaseStream.Seek(extraFieldLength + entry.CompressedLength, SeekOrigin.Current);
+                Span<uint> buffer = stackalloc uint[6];
+
+                // Let's try to match the smallest possible structure without signature (3 x 4 bytes)
+                buffer[0] = reader.ReadUInt32();
+                buffer[1] = reader.ReadUInt32();
+                buffer[2] = reader.ReadUInt32();
+
+                if (TestMatch(entry, DataDescriptorSignature, buffer[0], buffer[1], buffer[2]))
                 {
-                    wasDataDescriptorSignatureRead = true;
-                    seekSize = 4;
+                    return true;
                 }
 
-                bool is64bit = entry._versionToExtract >= ZipVersionNeededValues.Zip64;
-                seekSize += (is64bit ? 8 : 4) * 2;   // if Zip64 read by 8 bytes else 4 bytes 2 times (compressed and uncompressed size)
+                // If another 4 bytes are available
+                if (reader.BaseStream.Length < reader.BaseStream.Position + 4)
+                {
+                    return false;
+                }
+                // Let's try to match the next structure with signature (4 x 4 bytes)
+                buffer[3] = reader.ReadUInt32();
+                if (TestMatch(entry, buffer[0], buffer[1], buffer[2], buffer[3]))
+                {
+                    return true;
+                }
 
-                if (reader.BaseStream.Length < reader.BaseStream.Position + seekSize)
+                // At this point prior to trying to match 64 bit structures we need to make sure that version is high enough
+                if (entry._versionToExtract < ZipVersionNeededValues.Zip64)
                 {
                     return false;
                 }
 
-                // dataDescriptorSignature is optional, if it was the DataDescriptorSignature we need to skip CRC 4 bytes else we can assume CRC is alreadyskipped
-                if (wasDataDescriptorSignatureRead)
-                    reader.BaseStream.Seek(4, SeekOrigin.Current);
+                // If another 4 bytes available, let's try to match the 64 bit structure without signature
+                if (reader.BaseStream.Length < reader.BaseStream.Position + 4)
+                {
+                    return false;
+                }
+                buffer[4] = reader.ReadUInt32();
+                if (TestMatch(entry, DataDescriptorSignature, buffer[0], AsUInt64(buffer, 1), AsUInt64(buffer, 3)))
+                {
+                    return true;
+                }
 
-                if (is64bit)
+                // If another 4 bytes available, let's try to match the 64 bit structure with signature
+                if (reader.BaseStream.Length < reader.BaseStream.Position + 4)
                 {
-                    compressedSize = reader.ReadInt64();
-                    uncompressedSize = reader.ReadInt64();
+                    return false;
                 }
-                else
+                buffer[5] = reader.ReadUInt32();
+                if (TestMatch(entry, buffer[0], buffer[1], AsUInt64(buffer, 2), AsUInt64(buffer, 4)))
                 {
-                    compressedSize = reader.ReadInt32();
-                    uncompressedSize = reader.ReadInt32();
+                    return true;
                 }
-                reader.BaseStream.Seek(-seekSize - entry.CompressedLength - 4, SeekOrigin.Current); // Seek back to the beginning of compressed stream
             }
 
             if (entry.CompressedLength != compressedSize)
@@ -534,6 +581,17 @@ public static bool TryValidateBlock(BinaryReader reader, ZipArchiveEntry entry)
 
             return true;
         }
+
+        private static ulong AsUInt64(Span<uint> bytes, int start)
+        {
+            return (ulong)bytes[start] + (((ulong)bytes[start+1]) << 32);
+        }
+
+        private static bool TestMatch(ZipArchiveEntry entry, uint suspectSignature, uint suspectCrc32, ulong suspectCompressedLength, ulong suspectUncompressedLength)
+        {
+            return (DataDescriptorSignature == suspectSignature && (ulong)entry.CompressedLength == suspectCompressedLength
+                && (ulong)entry.Length == suspectUncompressedLength && entry.Crc32 == suspectCrc32);
+        }
     }
 
     internal struct ZipCentralDirectoryFileHeader

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

@@ -17,6 +17,7 @@ internal static class ZipHelper
         internal const int ValidZipDate_YearMax = 2107;
 
         private static readonly DateTime s_invalidDateIndicator = new DateTime(ValidZipDate_YearMin, 1, 1, 0, 0, 0);
+        internal static bool s_validateHeader = !GetSuppressHeaderValidation();
 
         internal static bool RequiresUnicode(string test)
         {
@@ -189,5 +190,15 @@ private static bool SeekBackwardsAndRead(Stream stream, byte[] buffer, out int b
                 return true;
             }
         }
+
+        private static bool GetSuppressHeaderValidation()
+        {
+            if (AppContext.TryGetSwitch("System.Compression.ZipArchiveEntry.SuppressHeaderValidation", out bool suppressValidation))
+            {
+                return suppressValidation;
+            }
+
+            return false;
+        }
     }
 }

src/libraries/System.IO.Compression/tests/System.IO.Compression.Tests.csproj

@@ -1,6 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <TargetFrameworks>$(NetCoreAppCurrent)-Windows_NT;$(NetCoreAppCurrent)-Unix</TargetFrameworks>
+    <IncludeRemoteExecutor>true</IncludeRemoteExecutor>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="CompressionStreamUnitTests.Deflate.cs" />

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

@@ -6,6 +6,7 @@
 using System.Text;
 using System.Threading.Tasks;
 using Xunit;
+using Microsoft.DotNet.RemoteExecutor;
 
 namespace System.IO.Compression.Tests
 {
@@ -128,6 +129,69 @@ public static async Task ZipArchiveEntry_InvalidUpdate(string zipname)
             }
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Fix not shipped for .NET Framework.")]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/32316", TestRuntimes.Mono)]
+        public static void ZipArchiveEntry_AppContext_Switch_SuppressHeaderValidation(bool suppressValidation)
+        {
+            RemoteExecutor.Invoke((suppressString) =>
+            {
+                bool suppress = bool.Parse(suppressString);
+                MemoryStream stream = populateStream().Result;
+                PatchDataRelativeToFileName(Encoding.ASCII.GetBytes(s_tamperedFileName), stream, 8);  // patch uncompressed size in file header
+                using (ZipArchive archive = new ZipArchive(stream, ZipArchiveMode.Read))
+                {
+                    // Set the AppContext Switch
+                    AppContext.SetSwitch("System.Compression.ZipArchiveEntry.SuppressHeaderValidation", suppress);
+                    ZipArchiveEntry e = archive.GetEntry(s_tamperedFileName);
+                    if (suppress)
+                    {
+                        using (MemoryStream ms = new MemoryStream())
+                        using (Stream source = e.Open())
+                        {
+                            source.CopyTo(ms);
+                            Assert.Equal(ms.Position, ms.Length);  // Just making sure it was readable
+                        }
+                    }
+                    else
+                    {
+                        Assert.Throws<InvalidDataException>(() => e.Open());
+                    }
+                }
+            }, suppressValidation.ToString()).Dispose();
+        }
+
+        [Fact]
+        public static async Task LargeArchive_DataDescriptor_Read_NonZip64_FileLengthGreaterThanIntMax()
+        {
+            MemoryStream stream = await LocalMemoryStream.readAppFileAsync(strange("fileLengthGreaterIntLessUInt.zip"));
+
+            using (ZipArchive archive = new ZipArchive(stream, ZipArchiveMode.Read))
+            {
+                ZipArchiveEntry e = archive.GetEntry("large.bin");
+
+                Assert.Equal(3_600_000_000, e.Length);
+                Assert.Equal(3_499_028, e.CompressedLength);
+
+                using (Stream source = e.Open())
+                {
+                    byte[] buffer = new byte[s_bufferSize];
+                    int read = source.Read(buffer, 0, buffer.Length);   // We don't want to inflate this large archive entirely 
+                                                                        // just making sure it read successfully 
+                    Assert.Equal(s_bufferSize, read);
+                    foreach (byte b in buffer)
+                    {
+                        if (b != '0')
+                        {
+                            Assert.True(false, "The file should be all 0s");
+                        }
+                    }
+                }
+            }
+        }
+
         [Fact]
         [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Fix not shipped for .NET Framework.")]
         public static async Task ZipArchiveEntry_CorruptedStream_ReadMode_CopyTo_UpToUncompressedSize()