Remove local header compressed/uncomressed size validation vs central directory recor...
authorbuyaa-n <bunamnan@microsoft.com>
Fri, 21 Feb 2020 05:34:03 +0000 (21:34 -0800)
committerGitHub <noreply@github.com>
Fri, 21 Feb 2020 05:34:03 +0000 (21:34 -0800)
* Zip data descriptor validation update and AppContext switch option

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

index e48c9e7..866e1ab 100644 (file)
@@ -749,21 +749,10 @@ namespace System.IO.Compression
                 }
                 Debug.Assert(_archive.ArchiveReader != null);
                 _archive.ArchiveStream.Seek(_offsetOfLocalHeader, SeekOrigin.Begin);
-                if (needToUncompress && !needToLoadIntoMemory)
+                if (!ZipLocalFileHeader.TrySkipBlock(_archive.ArchiveReader))
                 {
-                    if (!ZipLocalFileHeader.TryValidateBlock(_archive.ArchiveReader, this))
-                    {
-                        message = SR.LocalFileHeaderCorrupt;
-                        return false;
-                    }
-                }
-                else
-                {
-                    if (!ZipLocalFileHeader.TrySkipBlock(_archive.ArchiveReader))
-                    {
-                        message = SR.LocalFileHeaderCorrupt;
-                        return false;
-                    }
+                    message = SR.LocalFileHeaderCorrupt;
+                    return false;
                 }
                 // when this property gets called, some duplicated work
                 if (OffsetOfCompressedData + _compressedSize > _archive.ArchiveStream.Length)
index f5028da..55b6188 100644 (file)
@@ -431,109 +431,6 @@ namespace System.IO.Compression
 
             return true;
         }
-
-        public static bool TryValidateBlock(BinaryReader reader, ZipArchiveEntry entry)
-        {
-            const int OffsetToFilename = 26; // from the point after the signature
-
-            if (reader.ReadUInt32() != SignatureConstant)
-                return false;
-
-            if (reader.BaseStream.Length < reader.BaseStream.Position + OffsetToFilename)
-                return false;
-
-            reader.BaseStream.Seek(2, SeekOrigin.Current); // skipping minimum version (using min version from central directory header)
-            uint dataDescriptorBit = reader.ReadUInt16() & (uint)ZipArchiveEntry.BitFlagValues.DataDescriptor;
-            reader.BaseStream.Seek(10, SeekOrigin.Current); // skipping bytes used for Compression method (2 bytes), last modification time and date (4 bytes) and CRC (4 bytes)
-            long compressedSize = reader.ReadUInt32();
-            long uncompressedSize = reader.ReadUInt32();
-            int filenameLength = reader.ReadUInt16();
-            uint extraFieldLength = reader.ReadUInt16();
-
-            if (reader.BaseStream.Length < reader.BaseStream.Position + filenameLength + extraFieldLength)
-                return false;
-
-            reader.BaseStream.Seek(filenameLength, SeekOrigin.Current);  // skipping Filename
-            long endExtraFields = reader.BaseStream.Position + extraFieldLength;
-
-            if (dataDescriptorBit == 0)
-            {
-                bool isUncompressedSizeInZip64 = uncompressedSize == ZipHelper.Mask32Bit;
-                bool isCompressedSizeInZip64 = compressedSize == ZipHelper.Mask32Bit;
-                Zip64ExtraField zip64;
-
-                // Ideally we should also check if the minimumVersion is 64 bit or above, but there could zip files for which this version is not set correctly
-                if (isUncompressedSizeInZip64 || isCompressedSizeInZip64)
-                {
-                    zip64 = Zip64ExtraField.GetJustZip64Block(new SubReadStream(reader.BaseStream, reader.BaseStream.Position, extraFieldLength), isUncompressedSizeInZip64, isCompressedSizeInZip64, false, false);
-
-                    if (zip64.UncompressedSize != null)
-                    {
-                        uncompressedSize = zip64.UncompressedSize.Value;
-                    }
-
-                    if (zip64.CompressedSize != null)
-                    {
-                        compressedSize = zip64.CompressedSize.Value;
-                    }
-                }
-
-                reader.BaseStream.AdvanceToPosition(endExtraFields);
-            }
-            else
-            {
-                if (reader.BaseStream.Length < reader.BaseStream.Position + extraFieldLength + entry.CompressedLength + 4)
-                {
-                    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)
-                {
-                    wasDataDescriptorSignatureRead = true;
-                    seekSize = 4;
-                }
-
-                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 (reader.BaseStream.Length < reader.BaseStream.Position + seekSize)
-                {
-                    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 (is64bit)
-                {
-                    compressedSize = reader.ReadInt64();
-                    uncompressedSize = reader.ReadInt64();
-                }
-                else
-                {
-                    compressedSize = reader.ReadInt32();
-                    uncompressedSize = reader.ReadInt32();
-                }
-                reader.BaseStream.Seek(-seekSize - entry.CompressedLength - 4, SeekOrigin.Current); // Seek back to the beginning of compressed stream
-            }
-
-            if (entry.CompressedLength != compressedSize)
-            {
-                return false;
-            }
-
-            if (entry.Length != uncompressedSize)
-            {
-                return false;
-            }
-
-            return true;
-        }
     }
 
     internal struct ZipCentralDirectoryFileHeader
index d910a62..b6f7747 100644 (file)
@@ -129,6 +129,35 @@ namespace System.IO.Compression.Tests
         }
 
         [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 '0's, but found '{(char)b}'");
+                        }
+                    }
+                }
+            }
+        }
+
+        [Fact]
         [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Fix not shipped for .NET Framework.")]
         public static async Task ZipArchiveEntry_CorruptedStream_ReadMode_CopyTo_UpToUncompressedSize()
         {
@@ -190,7 +219,6 @@ namespace System.IO.Compression.Tests
         }
 
         [Fact]
-
         public static void ZipArchiveEntry_CorruptedStream_EnsureNoExtraBytesReadOrOverWritten()
         {
             MemoryStream stream = populateStream().Result;
@@ -327,55 +355,6 @@ namespace System.IO.Compression.Tests
         }
 
         [Fact]
-        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Fix not shipped for .NET Framework.")]
-        public static async Task ZipArchive_CorruptedLocalHeader_UncompressedSize_NotMatchWithCentralDirectory()
-        {
-            MemoryStream stream = await LocalMemoryStream.readAppFileAsync(zfile("normal.zip"));
-
-            PatchDataRelativeToFileName(Encoding.ASCII.GetBytes(s_tamperedFileName), stream, 8);  // patch uncompressed size in file header
-
-            using (ZipArchive archive = new ZipArchive(stream, ZipArchiveMode.Read))
-            {
-                ZipArchiveEntry e = archive.GetEntry(s_tamperedFileName);
-                Assert.Throws<InvalidDataException>(() => e.Open());
-            }
-        }
-
-        [Fact]
-        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Fix not shipped for .NET Framework.")]
-        public static async Task ZipArchive_CorruptedLocalHeader_CompressedSize_NotMatchWithCentralDirectory()
-        {
-            MemoryStream stream = await LocalMemoryStream.readAppFileAsync(zfile("normal.zip"));
-
-            PatchDataRelativeToFileName(Encoding.ASCII.GetBytes(s_tamperedFileName), stream, 12);  // patch compressed size in file header
-
-            using (ZipArchive archive = new ZipArchive(stream, ZipArchiveMode.Read))
-            {
-                ZipArchiveEntry e = archive.GetEntry(s_tamperedFileName);
-                Assert.Throws<InvalidDataException>(() => e.Open());
-            }
-        }
-
-        [Fact]
-        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, ".NET Framework does not allow unseekable streams.")]
-        public static async Task ZipArchive_Unseekable_Corrupted_FileDescriptor_NotMatchWithCentralDirectory()
-        {
-            using (var s = new MemoryStream())
-            {
-                var testStream = new WrappedStream(s, false, true, false, null);
-                await CreateFromDir(zfolder("normal"), testStream, ZipArchiveMode.Create);
-
-                PatchDataDescriptorRelativeToFileName(Encoding.ASCII.GetBytes(s_tamperedFileName), s, 8);  // patch uncompressed size in file descriptor
-
-                using (ZipArchive archive = new ZipArchive(s, ZipArchiveMode.Read))
-                {
-                    ZipArchiveEntry e = archive.GetEntry(s_tamperedFileName);
-                    Assert.Throws<InvalidDataException>(() => e.Open());
-                }
-            }
-        }
-
-        [Fact]
         public static async Task UpdateZipArchive_AppendTo_CorruptedFileEntry()
         {
             MemoryStream stream = await StreamHelpers.CreateTempCopyStream(zfile("normal.zip"));
@@ -506,23 +485,6 @@ namespace System.IO.Compression.Tests
             }
         }
 
-        private static int PatchDataDescriptorRelativeToFileName(byte[] fileNameInBytes, MemoryStream packageStream, int distance, int start = 0)
-        {
-            byte[] dataDescriptorSignature = BitConverter.GetBytes(0x08074B50);
-            byte[] buffer = packageStream.GetBuffer();
-            int startOfName = FindSequenceIndex(fileNameInBytes, buffer, start);
-            int startOfDataDescriptor = FindSequenceIndex(dataDescriptorSignature, buffer, startOfName);
-            var startOfUpdatingData = startOfDataDescriptor + distance;
-
-            // updating 4 byte data
-            buffer[startOfUpdatingData] = 0;
-            buffer[startOfUpdatingData + 1] = 1;
-            buffer[startOfUpdatingData + 2] = 20;
-            buffer[startOfUpdatingData + 3] = 0;
-
-            return startOfName;
-        }
-
         private static int PatchDataRelativeToFileName(byte[] fileNameInBytes, MemoryStream packageStream, int distance, int start = 0)
         {
             var buffer = packageStream.GetBuffer();