Add strict validation to System.IO.Compression.GZipStream

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

@@ -58,6 +58,7 @@ public GZipStream(System.IO.Stream stream, System.IO.Compression.CompressionLeve
         public GZipStream(System.IO.Stream stream, System.IO.Compression.CompressionLevel compressionLevel, bool leaveOpen) { }
         public GZipStream(System.IO.Stream stream, System.IO.Compression.CompressionMode mode) { }
         public GZipStream(System.IO.Stream stream, System.IO.Compression.CompressionMode mode, bool leaveOpen) { }
+        public GZipStream(System.IO.Stream stream, System.IO.Compression.CompressionMode mode, bool leaveOpen, bool strictValidation) { }
         public System.IO.Stream BaseStream { get { throw null; } }
         public override bool CanRead { get { throw null; } }
         public override bool CanSeek { get { throw null; } }

src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs

@@ -17,6 +17,7 @@ public partial class DeflateStream : Stream
         private Stream _stream;
         private CompressionMode _mode;
         private bool _leaveOpen;
+        private bool _strictValidation;
         private Inflater? _inflater;
         private Deflater? _deflater;
         private byte[]? _buffer;
@@ -49,7 +50,7 @@ public DeflateStream(Stream stream, CompressionLevel compressionLevel, bool leav
         /// Internal constructor to check stream validity and call the correct initialization function depending on
         /// the value of the CompressionMode given.
         /// </summary>
-        internal DeflateStream(Stream stream, CompressionMode mode, bool leaveOpen, int windowBits, long uncompressedSize = -1)
+        internal DeflateStream(Stream stream, CompressionMode mode, bool leaveOpen, int windowBits, long uncompressedSize = -1, bool strictValidation = false)
         {
             if (stream == null)
                 throw new ArgumentNullException(nameof(stream));
@@ -60,10 +61,11 @@ internal DeflateStream(Stream stream, CompressionMode mode, bool leaveOpen, int
                     if (!stream.CanRead)
                         throw new ArgumentException(SR.NotSupported_UnreadableStream, nameof(stream));
 
-                    _inflater = new Inflater(windowBits, uncompressedSize);
+                    _inflater = new Inflater(windowBits, uncompressedSize, strictValidation);
                     _stream = stream;
                     _mode = CompressionMode.Decompress;
                     _leaveOpen = leaveOpen;
+                    _strictValidation = strictValidation;
                     break;
 
                 case CompressionMode.Compress:
@@ -266,6 +268,10 @@ internal int ReadCore(Span<byte> buffer)
             while (true)
             {
                 int bytesRead = _inflater.Inflate(buffer.Slice(totalRead));
+                if (bytesRead == -1 && _strictValidation)
+                {
+                    throw new InvalidDataException(SR.GenericInvalidData);
+                }
                 totalRead += bytesRead;
                 if (totalRead == buffer.Length)
                 {
@@ -858,11 +864,15 @@ public async Task CopyFromSourceToDestinationAsync()
                         {
                             await _destination.WriteAsync(new ReadOnlyMemory<byte>(_arrayPoolBuffer, 0, bytesRead), _cancellationToken).ConfigureAwait(false);
                         }
-                        else if (_deflateStream._inflater.NeedsInput())
+                        else if (_deflateStream._inflater.NeedsInput() && bytesRead == 0)
                         {
                             // only break if we read 0 and ran out of input, if input is still available it may be another GZip payload
                             break;
                         }
+                        else if (bytesRead == -1 && _deflateStream._strictValidation)
+                        {
+                            throw new InvalidDataException(SR.GenericInvalidData);
+                        }
                     }
 
                     // Now, use the source stream's CopyToAsync to push directly to our inflater via this helper stream
@@ -890,11 +900,15 @@ public void CopyFromSourceToDestination()
                         {
                             _destination.Write(_arrayPoolBuffer, 0, bytesRead);
                         }
-                        else if (_deflateStream._inflater.NeedsInput())
+                        else if (_deflateStream._inflater.NeedsInput() && bytesRead == 0)
                         {
                             // only break if we read 0 and ran out of input, if input is still available it may be another GZip payload
                             break;
                         }
+                        else if (bytesRead == -1 && _deflateStream._strictValidation)
+                        {
+                            throw new InvalidDataException(SR.GenericInvalidData);
+                        }
                     }
 
                     // Now, use the source stream's CopyToAsync to push directly to our inflater via this helper stream
@@ -935,11 +949,15 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc
                     {
                         await _destination.WriteAsync(new ReadOnlyMemory<byte>(_arrayPoolBuffer, 0, bytesRead), cancellationToken).ConfigureAwait(false);
                     }
-                    else if (_deflateStream._inflater.NeedsInput())
+                    else if (_deflateStream._inflater.NeedsInput() && bytesRead == 0)
                     {
                         // only break if we read 0 and ran out of input, if input is still available it may be another GZip payload
                         break;
                     }
+                    else if (bytesRead == -1 && _deflateStream._strictValidation)
+                    {
+                        throw new InvalidDataException(SR.GenericInvalidData);
+                    }
                 }
             }
 
@@ -972,11 +990,15 @@ public override void Write(byte[] buffer, int offset, int count)
                     {
                         _destination.Write(_arrayPoolBuffer, 0, bytesRead);
                     }
-                    else if (_deflateStream._inflater.NeedsInput())
+                    else if (_deflateStream._inflater.NeedsInput() && bytesRead == 0)
                     {
                         // only break if we read 0 and ran out of input, if input is still available it may be another GZip payload
                         break;
                     }
+                    else if (bytesRead == -1 && _deflateStream._strictValidation)
+                    {
+                        throw new InvalidDataException(SR.GenericInvalidData);
+                    }
                 }
             }
 

src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Inflater.cs

@@ -22,21 +22,23 @@ internal sealed class Inflater : IDisposable
         private ZLibNative.ZLibStreamHandle _zlibStream;    // The handle to the primary underlying zlib stream
         private GCHandle _inputBufferHandle;                // The handle to the buffer that provides input to _zlibStream
         private readonly long _uncompressedSize;
+        private readonly bool _strictValidation;
         private long _currentInflatedCount;
 
         private object SyncLock => this;                    // Used to make writing to unmanaged structures atomic
 
         /// <summary>
         /// Initialized the Inflater with the given windowBits size
         /// </summary>
-        internal Inflater(int windowBits, long uncompressedSize = -1)
+        internal Inflater(int windowBits, long uncompressedSize = -1, bool strictValidation = false)
         {
             Debug.Assert(windowBits >= MinWindowBits && windowBits <= MaxWindowBits);
             _finished = false;
             _isDisposed = false;
             _windowBits = windowBits;
             InflateInit(windowBits);
             _uncompressedSize = uncompressedSize;
+            _strictValidation = strictValidation;
         }
 
         public int AvailableOutput => (int)_zlibStream.AvailOut;
@@ -119,7 +121,8 @@ public unsafe int InflateVerified(byte* bufPtr, int length)
 
         private unsafe void ReadOutput(byte* bufPtr, int length, out int bytesRead)
         {
-            if (ReadInflateOutput(bufPtr, length, ZLibNative.FlushCode.NoFlush, out bytesRead) == ZLibNative.ErrorCode.StreamEnd)
+            var result = ReadInflateOutput(bufPtr, length, ZLibNative.FlushCode.NoFlush, out bytesRead);
+            if (result == ZLibNative.ErrorCode.StreamEnd)
             {
                 if (!NeedsInput() && IsGzipStream() && _inputBufferHandle.IsAllocated)
                 {
@@ -130,6 +133,10 @@ private unsafe void ReadOutput(byte* bufPtr, int length, out int bytesRead)
                     _finished = true;
                 }
             }
+            else if (result == ZLibNative.ErrorCode.BufError && _strictValidation)
+            {
+                bytesRead = -1;
+            }
         }
 
         /// <summary>

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

@@ -20,6 +20,11 @@ public GZipStream(Stream stream, CompressionMode mode, bool leaveOpen)
             _deflateStream = new DeflateStream(stream, mode, leaveOpen, ZLibNative.GZip_DefaultWindowBits);
         }
 
+        public GZipStream(Stream stream, CompressionMode mode, bool leaveOpen, bool strictValidation)
+        {
+            _deflateStream = new DeflateStream(stream, mode, leaveOpen, ZLibNative.GZip_DefaultWindowBits, strictValidation: strictValidation);
+        }
+
         // Implies mode = Compress
         public GZipStream(Stream stream, CompressionLevel compressionLevel) : this(stream, compressionLevel, leaveOpen: false)
         {

src/libraries/System.IO.Compression/tests/CompressionStreamUnitTests.Gzip.cs

@@ -2,6 +2,10 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Buffers;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using Xunit;
@@ -64,7 +68,7 @@ public async Task ConcatenatedEmptyGzipStreams()
             ArrayPool<byte>.Shared.Return(rentedBuffer);
 
             // use 3 buffers-full so that we can prime the stream with the first buffer-full,
-            // test that CopyTo successfully flushes this at the beginning of the operation, 
+            // test that CopyTo successfully flushes this at the beginning of the operation,
             // then populates the second buffer-full and reads its entirety despite every
             // payload being 0 length before it reads the final buffer-full.
             int minCompressedSize = 3 * actualBufferSize;
@@ -167,9 +171,9 @@ private async Task TestConcatenatedGzipStreams(int streamCount, TestScenario sce
             bool isCopy = scenario == TestScenario.Copy || scenario == TestScenario.CopyAsync;
 
             using (MemoryStream correctDecompressedOutput = new MemoryStream())
-            // For copy scenarios use a derived MemoryStream to avoid MemoryStream's Copy optimization 
+            // For copy scenarios use a derived MemoryStream to avoid MemoryStream's Copy optimization
             // that turns the Copy into a single Write passing the backing buffer
-            using (MemoryStream compressedStream = isCopy ? new DerivedMemoryStream() : new MemoryStream())  
+            using (MemoryStream compressedStream = isCopy ? new DerivedMemoryStream() : new MemoryStream())
             using (MemoryStream decompressorOutput = new MemoryStream())
             {
                 for (int i = 0; i < streamCount; i++)
@@ -281,6 +285,181 @@ public void DerivedStream_ReadWriteSpan_UsesReadWriteArray()
             }
         }
 
+        // https://stackoverflow.com/questions/9456563/gzipstream-doesnt-detect-corrupt-data-even-crc32-passes
+        [Fact]
+        public void StrictValidation()
+        {
+            var data = Enumerable.Range(0, 32).Select(i => (byte)i).ToArray();
+            byte[] cmpData;
+
+            // Compress
+            using (var cmpStream = new MemoryStream())
+            {
+                using (var hgs = new GZipStream(cmpStream, CompressionMode.Compress))
+                {
+                    hgs.Write(data, 0, data.Length);
+                }
+                cmpData = cmpStream.ToArray();
+            }
+
+            int corruptBytesNotDetected = 0;
+            int corruptBytesDetected = 0;
+
+            // corrupt data byte by byte
+            for (var byteToCorrupt = 0; byteToCorrupt < cmpData.Length; byteToCorrupt++)
+            {
+                // corrupt the data
+                cmpData[byteToCorrupt]++;
+
+                using (var decomStream = new MemoryStream(cmpData))
+                {
+                    using (var hgs = new GZipStream(decomStream, CompressionMode.Decompress, false, strictValidation: true))
+                    {
+                        using (var reader = new StreamReader(hgs))
+                        {
+                            try
+                            {
+                                string sampleOut = reader.ReadToEnd();
+
+                                // if we get here, the corrupt data was not detected by GZipStream
+                                // ... okay so long as the correct data is extracted
+                                corruptBytesNotDetected++;
+                            }
+                            catch (InvalidDataException)
+                            {
+                                corruptBytesDetected++;
+                            }
+                        }
+                    }
+                }
+
+                // restore the data
+                cmpData[byteToCorrupt]--;
+            }
+
+            Assert.Equal(0, corruptBytesNotDetected);
+            Assert.Equal(52, corruptBytesDetected);
+        }
+
+        [Fact]
+        public void StrictValidation2()
+        {
+            var source = Enumerable.Range(0, 32).Select(i => (byte)i).ToArray();
+            var codec = new (Func<Stream, Stream> compress, Func<Stream, Stream> decompress)[]
+            {
+               (
+                    s => new System.IO.Compression.GZipStream(s, System.IO.Compression.CompressionLevel.Fastest),
+                    s => new System.IO.Compression.GZipStream(s, CompressionMode.Decompress, false, strictValidation: true)
+               )
+            };
+            string r = null;
+
+            try
+            {
+                r = RoundTrip(source, codec[0], Truncate);
+            }
+            catch(Exception ex)
+            {
+                Debug.WriteLine(ex.Message);
+            }
+
+            Assert.Equal("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", r);
+        }
+
+        private static string RoundTrip(
+         ReadOnlySpan<byte> source,
+         (Func<Stream, Stream> compress, Func<Stream, Stream> decompress) codec,
+         Func<ReadOnlyMemory<byte>, IEnumerable<(Stream input, bool? shouldBeOkay)>> peturbations)
+        {
+            byte[] data;
+            using (var compressed = new MemoryStream())
+            using (var gzip = codec.compress(compressed))
+            {
+                foreach (var b in source)
+                {
+                    gzip.WriteByte(b);
+                }
+                gzip.Dispose();
+                data = compressed.ToArray();
+            }
+
+            var actual = new StringBuilder();
+            foreach (var entry in peturbations(data))
+            {
+                try
+                {
+                    using (var input = entry.input)
+                    using (var gzip = codec.decompress(input))
+                    using (var roundtrip = new MemoryStream())
+                    {
+                        gzip.CopyTo(roundtrip);
+                        var result = roundtrip.ToArray();
+                    }
+                    actual.Append("/");
+                }
+                catch (Exception)
+                {
+                    actual.Append("x");
+                }
+            }
+
+            return actual.ToString();
+        }
+
+        private static IEnumerable<(Stream, bool? shouldBeLegal)> Truncate(ReadOnlyMemory<byte> source)
+        {
+            var data = source.ToArray();
+            for (int i = 0; i < data.Length; i++)
+            {
+                yield return (
+                    new MemoryStream(data, 0, i, writable: false),
+                    CanByteBeManipulatedSafely(data.Length, i, truncated: true));
+            }
+        }
+
+        private static bool? CanByteBeManipulatedSafely(int length, int index, bool truncated)
+        {
+            // https://tools.ietf.org/html/rfc1952.html#section-2.2
+            switch (index)
+            {
+                case 0: // ID1
+                case 1: // ID2
+                case 2: // CM (always gzip)
+                    return false;
+                //FLG some flags values may be illegal but ignoring for now
+                case 3:
+                    return truncated ? false : (bool?)null;
+                // MTIME
+                case 4:
+                case 5:
+                case 6:
+                case 7:
+                    return truncated ? false : true;
+                //XFL some flags values may be illegal but ignoring for now
+                case 8:
+                    return truncated ? false : (bool?)null;
+                // OS
+                case 9:
+                    return truncated ? false : (bool?)null;
+                default:
+                    // footer
+                    if (length > 18 && index > length - 4)
+                    {
+                        // isize - should be validated
+                        return false;
+                    }
+                    else if (length > 18 && index > length - 8)
+                    {
+                        // CRC32 - should be validated - technically certain manipulations could be undetected
+                        return false;
+                    }
+                    // depending on flags/xflags there may be header entries in here that are mutable
+                    // however if FLG.FHCRC was set thyen they aren't, so just assum none can be changed
+                    return false;
+            }
+        }
+
+
         private sealed class DerivedGZipStream : GZipStream
         {
             public bool ReadArrayInvoked = false, WriteArrayInvoked = false;