Merge pull request #1005 from SixLabors/issue/1004

Fix #1004

Author: JimBobSquarePants

src/ImageSharp/Formats/Png/PngDecoderCore.cs

@@ -5,6 +5,7 @@
 using System.Buffers.Binary;
 using System.Collections.Generic;
 using System.IO;
+using System.IO.Compression;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Text;
@@ -175,11 +176,7 @@ public Image<TPixel> Decode<TPixel>(Stream stream)
                                     this.InitializeImage(metadata, out image);
                                 }
 
-                                using (var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk))
-                                {
-                                    deframeStream.AllocateNewBytes(chunk.Length);
-                                    this.ReadScanlines(deframeStream.CompressedStream, image.Frames.RootFrame, pngMetadata);
-                                }
+                                this.ReadScanlines(chunk, image.Frames.RootFrame, pngMetadata);
 
                                 break;
                             case PngChunkType.Palette:
@@ -465,19 +462,25 @@ private int CalculateScanlineLength(int width)
         /// Reads the scanlines within the image.
         /// </summary>
         /// <typeparam name="TPixel">The pixel format.</typeparam>
-        /// <param name="dataStream">The <see cref="MemoryStream"/> containing data.</param>
+        /// <param name="chunk">The png chunk containing the compressed scanline data.</param>
         /// <param name="image"> The pixel data.</param>
         /// <param name="pngMetadata">The png metadata</param>
-        private void ReadScanlines<TPixel>(Stream dataStream, ImageFrame<TPixel> image, PngMetadata pngMetadata)
+        private void ReadScanlines<TPixel>(PngChunk chunk, ImageFrame<TPixel> image, PngMetadata pngMetadata)
             where TPixel : struct, IPixel<TPixel>
         {
-            if (this.header.InterlaceMethod == PngInterlaceMode.Adam7)
-            {
-                this.DecodeInterlacedPixelData(dataStream, image, pngMetadata);
-            }
-            else
+            using (var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk))
             {
-                this.DecodePixelData(dataStream, image, pngMetadata);
+                deframeStream.AllocateNewBytes(chunk.Length, true);
+                DeflateStream dataStream = deframeStream.CompressedStream;
+
+                if (this.header.InterlaceMethod == PngInterlaceMode.Adam7)
+                {
+                    this.DecodeInterlacedPixelData(dataStream, image, pngMetadata);
+                }
+                else
+                {
+                    this.DecodePixelData(dataStream, image, pngMetadata);
+                }
             }
         }
 
@@ -924,7 +927,11 @@ private void ReadCompressedTextChunk(PngMetadata metadata, ReadOnlySpan<byte> da
             }
 
             ReadOnlySpan<byte> compressedData = data.Slice(zeroIndex + 2);
-            metadata.TextData.Add(new PngTextData(name, this.UncompressTextData(compressedData, PngConstants.Encoding), string.Empty, string.Empty));
+
+            if (this.TryUncompressTextData(compressedData, PngConstants.Encoding, out string uncompressed))
+            {
+                metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty));
+            }
         }
 
         /// <summary>
@@ -987,7 +994,11 @@ private void ReadInternationalTextChunk(PngMetadata metadata, ReadOnlySpan<byte>
             if (compressionFlag == 1)
             {
                 ReadOnlySpan<byte> compressedData = data.Slice(dataStartIdx);
-                metadata.TextData.Add(new PngTextData(keyword, this.UncompressTextData(compressedData, PngConstants.TranslatedEncoding), language, translatedKeyword));
+
+                if (this.TryUncompressTextData(compressedData, PngConstants.TranslatedEncoding, out string uncompressed))
+                {
+                    metadata.TextData.Add(new PngTextData(keyword, uncompressed, language, translatedKeyword));
+                }
             }
             else
             {
@@ -1001,13 +1012,19 @@ private void ReadInternationalTextChunk(PngMetadata metadata, ReadOnlySpan<byte>
         /// </summary>
         /// <param name="compressedData">Compressed text data bytes.</param>
         /// <param name="encoding">The string encoding to use.</param>
-        /// <returns>A string.</returns>
-        private string UncompressTextData(ReadOnlySpan<byte> compressedData, Encoding encoding)
+        /// <param name="value">The uncompressed value.</param>
+        /// <returns>The <see cref="bool"/>.</returns>
+        private bool TryUncompressTextData(ReadOnlySpan<byte> compressedData, Encoding encoding, out string value)
         {
             using (var memoryStream = new MemoryStream(compressedData.ToArray()))
-            using (var inflateStream = new ZlibInflateStream(memoryStream, () => 0))
+            using (var inflateStream = new ZlibInflateStream(memoryStream))
             {
-                inflateStream.AllocateNewBytes(compressedData.Length);
+                if (!inflateStream.AllocateNewBytes(compressedData.Length, false))
+                {
+                    value = null;
+                    return false;
+                }
+
                 var uncompressedBytes = new List<byte>();
 
                 // Note: this uses the a buffer which is only 4 bytes long to read the stream, maybe allocating a larger buffer makes sense here.
@@ -1018,7 +1035,8 @@ private string UncompressTextData(ReadOnlySpan<byte> compressedData, Encoding en
                     bytesRead = inflateStream.CompressedStream.Read(this.buffer, 0, this.buffer.Length);
                 }
 
-                return encoding.GetString(uncompressedBytes.ToArray());
+                value = encoding.GetString(uncompressedBytes.ToArray());
+                return true;
             }
         }
 

src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs

@@ -20,14 +20,14 @@ internal sealed class ZlibInflateStream : Stream
         private static readonly byte[] ChecksumBuffer = new byte[4];
 
         /// <summary>
-        /// The inner raw memory stream
+        /// A default delegate to get more data from the inner stream.
         /// </summary>
-        private readonly Stream innerStream;
+        private static readonly Func<int> GetDataNoOp = () => 0;
 
         /// <summary>
-        /// The compressed stream sitting over the top of the deframer
+        /// The inner raw memory stream
         /// </summary>
-        private DeflateStream compressedStream;
+        private readonly Stream innerStream;
 
         /// <summary>
         /// A value indicating whether this instance of the given entity has been disposed.
@@ -55,8 +55,17 @@ internal sealed class ZlibInflateStream : Stream
         /// <summary>
         /// Initializes a new instance of the <see cref="ZlibInflateStream"/> class.
         /// </summary>
-        /// <param name="innerStream">The inner raw stream</param>
-        /// <param name="getData">A delegate to get more data from the inner stream</param>
+        /// <param name="innerStream">The inner raw stream.</param>
+        public ZlibInflateStream(Stream innerStream)
+            : this(innerStream, GetDataNoOp)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ZlibInflateStream"/> class.
+        /// </summary>
+        /// <param name="innerStream">The inner raw stream.</param>
+        /// <param name="getData">A delegate to get more data from the inner stream.</param>
         public ZlibInflateStream(Stream innerStream, Func<int> getData)
         {
             this.innerStream = innerStream;
@@ -76,31 +85,32 @@ public ZlibInflateStream(Stream innerStream, Func<int> getData)
         public override long Length => throw new NotSupportedException();
 
         /// <inheritdoc/>
-        public override long Position { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
+        public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); }
 
         /// <summary>
         /// Gets the compressed stream over the deframed inner stream
         /// </summary>
-        public DeflateStream CompressedStream => this.compressedStream;
+        public DeflateStream CompressedStream { get; private set; }
 
         /// <summary>
         /// Adds new bytes from a frame found in the original stream
         /// </summary>
         /// <param name="bytes">blabla</param>
-        public void AllocateNewBytes(int bytes)
+        /// <param name="isCriticalChunk">Whether the chunk to be inflated is a critical chunk.</param>
+        /// <returns>The <see cref="bool"/>.</returns>
+        public bool AllocateNewBytes(int bytes, bool isCriticalChunk)
         {
             this.currentDataRemaining = bytes;
-            if (this.compressedStream is null)
+            if (this.CompressedStream is null)
             {
-                this.InitializeInflateStream();
+                return this.InitializeInflateStream(isCriticalChunk);
             }
+
+            return true;
         }
 
         /// <inheritdoc/>
-        public override void Flush()
-        {
-            throw new NotSupportedException();
-        }
+        public override void Flush() => throw new NotSupportedException();
 
         /// <inheritdoc/>
         public override int ReadByte()
@@ -182,10 +192,10 @@ protected override void Dispose(bool disposing)
             if (disposing)
             {
                 // dispose managed resources
-                if (this.compressedStream != null)
+                if (this.CompressedStream != null)
                 {
-                    this.compressedStream.Dispose();
-                    this.compressedStream = null;
+                    this.CompressedStream.Dispose();
+                    this.CompressedStream = null;
                 }
             }
 
@@ -197,7 +207,7 @@ protected override void Dispose(bool disposing)
             this.isDisposed = true;
         }
 
-        private void InitializeInflateStream()
+        private bool InitializeInflateStream(bool isCriticalChunk)
         {
             // Read the zlib header : http://tools.ietf.org/html/rfc1950
             // CMF(Compression Method and flags)
@@ -215,7 +225,7 @@ private void InitializeInflateStream()
             this.currentDataRemaining -= 2;
             if (cmf == -1 || flag == -1)
             {
-                return;
+                return false;
             }
 
             if ((cmf & 0x0F) == 8)
@@ -225,14 +235,28 @@ private void InitializeInflateStream()
 
                 if (cinfo > 7)
                 {
-                    // Values of CINFO above 7 are not allowed in RFC1950.
-                    // CINFO is not defined in this specification for CM not equal to 8.
-                    throw new ImageFormatException($"Invalid window size for ZLIB header: cinfo={cinfo}");
+                    if (isCriticalChunk)
+                    {
+                        // Values of CINFO above 7 are not allowed in RFC1950.
+                        // CINFO is not defined in this specification for CM not equal to 8.
+                        throw new ImageFormatException($"Invalid window size for ZLIB header: cinfo={cinfo}");
+                    }
+                    else
+                    {
+                        return false;
+                    }
                 }
             }
             else
             {
-                throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}");
+                if (isCriticalChunk)
+                {
+                    throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}");
+                }
+                else
+                {
+                    return false;
+                }
             }
 
             // The preset dictionary.
@@ -246,7 +270,9 @@ private void InitializeInflateStream()
             }
 
             // Initialize the deflate Stream.
-            this.compressedStream = new DeflateStream(this, CompressionMode.Decompress, true);
+            this.CompressedStream = new DeflateStream(this, CompressionMode.Decompress, true);
+
+            return true;
         }
     }
 }

tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs

@@ -40,7 +40,8 @@ public partial class PngDecoderTests
             TestImages.Png.GrayAlpha8Bit,
             TestImages.Png.Gray1BitTrans,
             TestImages.Png.Bad.ZlibOverflow,
-            TestImages.Png.Bad.ZlibOverflow2
+            TestImages.Png.Bad.ZlibOverflow2,
+            TestImages.Png.Bad.ZlibZtxtBadHeader,
         };
 
         public static readonly string[] TestImages48Bpp =

tests/ImageSharp.Tests/TestImages.cs

@@ -90,6 +90,7 @@ public static class Bad
                 public const string CorruptedChunk = "Png/big-corrupted-chunk.png";
                 public const string ZlibOverflow = "Png/zlib-overflow.png";
                 public const string ZlibOverflow2 = "Png/zlib-overflow2.png";
+                public const string ZlibZtxtBadHeader = "Png/zlib-ztxt-bad-header.png";
             }
 
             public static readonly string[] All =