From 8ea98385cef8e68317f9249f3e85ee2d6b40f352 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 16 Dec 2021 13:13:06 +0100 Subject: [PATCH 1/3] Verify CRC of IDAT chunk is correct --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index ffaa9d567f..f81cbca211 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1206,16 +1206,16 @@ private bool TryReadChunk(out PngChunk chunk) 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 @@ private bool TryReadChunk(out PngChunk chunk) 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; } From 3310fa6ffdfd283b834c91cdfa76436bdf07332e Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 16 Dec 2021 13:21:27 +0100 Subject: [PATCH 2/3] Add test for wrong CRC in IDAT chunk --- .../Formats/Png/PngDecoderTests.cs | 33 ++++++++++++++----- tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Png/xcsn0g01.png | 3 ++ 3 files changed, 28 insertions(+), 9 deletions(-) create mode 100644 tests/Images/Input/Png/xcsn0g01.png diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index d63dad8d4b..de1c47d417 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -255,7 +255,7 @@ public void Identify(string imagePath, int expectedPixelSize) public void Decode_MissingDataChunk_ThrowsException(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -271,7 +271,7 @@ public void Decode_MissingDataChunk_ThrowsException(TestImageProvider(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -287,7 +287,7 @@ public void Decode_InvalidBitDepth_ThrowsException(TestImageProvider(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -297,13 +297,28 @@ public void Decode_InvalidColorType_ThrowsException(TestImageProvider(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + }); + 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(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -319,7 +334,7 @@ public void Issue1014_DataSplitOverMultipleIDatChunks(TestImageProvider< public void Issue1177_CRC_Omitted(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -335,7 +350,7 @@ public void Issue1177_CRC_Omitted(TestImageProvider provider) public void Issue1127(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -351,7 +366,7 @@ public void Issue1127(TestImageProvider provider) public void Issue1047(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -372,7 +387,7 @@ public void Issue1047(TestImageProvider provider) public void Issue1765(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -388,7 +403,7 @@ public void Issue1765(TestImageProvider provider) public void Issue410_MalformedApplePng(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index f43c0c9bd6..b87ffc7420 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -120,6 +120,7 @@ public static class Png 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. diff --git a/tests/Images/Input/Png/xcsn0g01.png b/tests/Images/Input/Png/xcsn0g01.png new file mode 100644 index 0000000000..6908a107c4 --- /dev/null +++ b/tests/Images/Input/Png/xcsn0g01.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:71e4b2826f61556eda39f3a93c8769b14d3ac90f135177b9373061199dbef39a +size 164 From f3a5ba0b4d94b2e8422416a733e3d7d272fa0786 Mon Sep 17 00:00:00 2001 From: Brian Popow <38701097+brianpopow@users.noreply.github.com> Date: Thu, 16 Dec 2021 15:45:23 +0100 Subject: [PATCH 3/3] Review suggestion Co-authored-by: Anton Firszov --- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index de1c47d417..c29f8c5891 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -302,11 +302,10 @@ public void Decode_InvalidColorType_ThrowsException(TestImageProvider(TestImageProvider provider) where TPixel : unmanaged, IPixel { - Exception ex = Record.Exception( + InvalidImageContentException ex = Assert.Throws( () => { using Image image = provider.GetImage(PngDecoder); - image.DebugSave(provider); }); Assert.NotNull(ex); Assert.Contains("CRC Error. PNG IDAT chunk is corrupt!", ex.Message);