Merge pull request #1898 from SixLabors/bp/png_idatcrc

PNG: Verify CRC of IDAT chunk is correct

src/ImageSharp/Formats/Png/PngDecoderCore.cs

@@ -1206,16 +1206,16 @@ namespace SixLabors.ImageSharp.Formats.Png
 
             PngChunkType type = this.ReadChunkType();
 
-            // NOTE: Reading the Data chunk is the responsible of the caller
             // If we're reading color metadata only we're only interested in the IHDR and tRNS chunks.
             // We can skip all other chunk data in the stream for better performance.
-            if (type == PngChunkType.Data || (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency))
+            if (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency)
             {
                 chunk = new PngChunk(length, type);
 
                 return true;
             }
 
+            long pos = this.currentStream.Position;
             chunk = new PngChunk(
                 length: length,
                 type: type,
@@ -1223,6 +1223,13 @@ namespace SixLabors.ImageSharp.Formats.Png
 
             this.ValidateChunk(chunk);
 
+            // Restore the stream position for IDAT chunks, because it will be decoded later and
+            // was only read to verifying the CRC is correct.
+            if (type == PngChunkType.Data)
+            {
+                this.currentStream.Position = pos;
+            }
+
             return true;
         }
 

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

@@ -255,7 +255,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
         public void Decode_MissingDataChunk_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            System.Exception ex = Record.Exception(
+            Exception ex = Record.Exception(
                 () =>
                 {
                     using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -271,7 +271,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
         public void Decode_InvalidBitDepth_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            System.Exception ex = Record.Exception(
+            Exception ex = Record.Exception(
                 () =>
                 {
                     using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -287,7 +287,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
         public void Decode_InvalidColorType_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            System.Exception ex = Record.Exception(
+            Exception ex = Record.Exception(
                 () =>
                 {
                     using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -297,13 +297,27 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
             Assert.Contains("Invalid or unsupported color type", ex.Message);
         }
 
+        [Theory]
+        [WithFile(TestImages.Png.Bad.WrongCrcDataChunk, PixelTypes.Rgba32)]
+        public void Decode_InvalidDataChunkCrc_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            InvalidImageContentException ex = Assert.Throws<InvalidImageContentException>(
+                () =>
+                {
+                    using Image<TPixel> image = provider.GetImage(PngDecoder);
+                });
+            Assert.NotNull(ex);
+            Assert.Contains("CRC Error. PNG IDAT chunk is corrupt!", ex.Message);
+        }
+
         // https://github.com/SixLabors/ImageSharp/issues/1014
         [Theory]
         [WithFileCollection(nameof(TestImagesIssue1014), PixelTypes.Rgba32)]
         public void Issue1014_DataSplitOverMultipleIDatChunks<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            System.Exception ex = Record.Exception(
+            Exception ex = Record.Exception(
                 () =>
                 {
                     using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -319,7 +333,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
         public void Issue1177_CRC_Omitted<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            System.Exception ex = Record.Exception(
+            Exception ex = Record.Exception(
                 () =>
                 {
                     using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -335,7 +349,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
         public void Issue1127<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            System.Exception ex = Record.Exception(
+            Exception ex = Record.Exception(
                 () =>
                 {
                     using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -351,7 +365,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
         public void Issue1047<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            System.Exception ex = Record.Exception(
+            Exception ex = Record.Exception(
                 () =>
                 {
                     using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -372,7 +386,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
         public void Issue1765<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            System.Exception ex = Record.Exception(
+            Exception ex = Record.Exception(
                 () =>
                 {
                     using Image<TPixel> image = provider.GetImage(PngDecoder);
@@ -388,7 +402,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
         public void Issue410_MalformedApplePng<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            System.Exception ex = Record.Exception(
+            Exception ex = Record.Exception(
                 () =>
                 {
                     using Image<TPixel> image = provider.GetImage(PngDecoder);

tests/ImageSharp.Tests/TestImages.cs

@@ -120,6 +120,7 @@ namespace SixLabors.ImageSharp.Tests
             public static class Bad
             {
                 public const string MissingDataChunk = "Png/xdtn0g01.png";
+                public const string WrongCrcDataChunk = "Png/xcsn0g01.png";
                 public const string CorruptedChunk = "Png/big-corrupted-chunk.png";
 
                 // Zlib errors.

tests/Images/Input/Png/xcsn0g01.png

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:71e4b2826f61556eda39f3a93c8769b14d3ac90f135177b9373061199dbef39a
+size 164