diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index e5fa5fbb72..6ec0df9ad6 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -41,9 +41,13 @@ public PngChunk(int length, PngChunkType type, IMemoryOwner data = null) /// /// Gets a value indicating whether the given chunk is critical to decoding /// - public bool IsCritical => - this.Type is PngChunkType.Header or - PngChunkType.Palette or - PngChunkType.Data or - PngChunkType.FrameData; + /// The chunk CRC handling behavior. + public bool IsCritical(PngCrcChunkHandling handling) + => handling switch + { + PngCrcChunkHandling.IgnoreNone => true, + PngCrcChunkHandling.IgnoreNonCritical => this.Type is PngChunkType.Header or PngChunkType.Palette or PngChunkType.Data or PngChunkType.FrameData, + PngCrcChunkHandling.IgnoreData => this.Type is PngChunkType.Header or PngChunkType.Palette, + _ => false, + }; } diff --git a/src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs b/src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs new file mode 100644 index 0000000000..264d737fdc --- /dev/null +++ b/src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs @@ -0,0 +1,30 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +namespace SixLabors.ImageSharp.Formats.Png; + +/// +/// Specifies how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG. +/// +public enum PngCrcChunkHandling +{ + /// + /// Do not ignore any CRC chunk errors. + /// + IgnoreNone, + + /// + /// Ignore CRC errors in non critical chunks. + /// + IgnoreNonCritical, + + /// + /// Ignore CRC errors in data chunks. + /// + IgnoreData, + + /// + /// Ignore CRC errors in all chunks. + /// + IgnoreAll +} diff --git a/src/ImageSharp/Formats/Png/PngDecoder.cs b/src/ImageSharp/Formats/Png/PngDecoder.cs index d226451389..4a7ba3f961 100644 --- a/src/ImageSharp/Formats/Png/PngDecoder.cs +++ b/src/ImageSharp/Formats/Png/PngDecoder.cs @@ -8,7 +8,7 @@ namespace SixLabors.ImageSharp.Formats.Png; /// /// Decoder for generating an image out of a png encoded stream. /// -public sealed class PngDecoder : ImageDecoder +public sealed class PngDecoder : SpecializedImageDecoder { private PngDecoder() { @@ -25,31 +25,31 @@ protected override ImageInfo Identify(DecoderOptions options, Stream stream, Can Guard.NotNull(options, nameof(options)); Guard.NotNull(stream, nameof(stream)); - return new PngDecoderCore(options).Identify(options.Configuration, stream, cancellationToken); + return new PngDecoderCore(new PngDecoderOptions() { GeneralOptions = options }).Identify(options.Configuration, stream, cancellationToken); } /// - protected override Image Decode(DecoderOptions options, Stream stream, CancellationToken cancellationToken) + protected override Image Decode(PngDecoderOptions options, Stream stream, CancellationToken cancellationToken) { Guard.NotNull(options, nameof(options)); Guard.NotNull(stream, nameof(stream)); PngDecoderCore decoder = new(options); - Image image = decoder.Decode(options.Configuration, stream, cancellationToken); + Image image = decoder.Decode(options.GeneralOptions.Configuration, stream, cancellationToken); - ScaleToTargetSize(options, image); + ScaleToTargetSize(options.GeneralOptions, image); return image; } /// - protected override Image Decode(DecoderOptions options, Stream stream, CancellationToken cancellationToken) + protected override Image Decode(PngDecoderOptions options, Stream stream, CancellationToken cancellationToken) { Guard.NotNull(options, nameof(options)); Guard.NotNull(stream, nameof(stream)); PngDecoderCore decoder = new(options, true); - ImageInfo info = decoder.Identify(options.Configuration, stream, cancellationToken); + ImageInfo info = decoder.Identify(options.GeneralOptions.Configuration, stream, cancellationToken); stream.Position = 0; PngMetadata meta = info.Metadata.GetPngMetadata(); @@ -99,4 +99,7 @@ protected override Image Decode(DecoderOptions options, Stream stream, Cancellat return this.Decode(options, stream, cancellationToken); } } + + /// + protected override PngDecoderOptions CreateDefaultSpecializedOptions(DecoderOptions options) => new PngDecoderOptions() { GeneralOptions = options }; } diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 49490ecd54..84eea9a5a5 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -114,27 +114,34 @@ internal sealed class PngDecoderCore : IImageDecoderInternals /// private PngChunk? nextChunk; + /// + /// How to handle CRC errors. + /// + private readonly PngCrcChunkHandling pngCrcChunkHandling; + /// /// Initializes a new instance of the class. /// /// The decoder options. - public PngDecoderCore(DecoderOptions options) + public PngDecoderCore(PngDecoderOptions options) { - this.Options = options; - this.configuration = options.Configuration; - this.maxFrames = options.MaxFrames; - this.skipMetadata = options.SkipMetadata; + this.Options = options.GeneralOptions; + this.configuration = options.GeneralOptions.Configuration; + this.maxFrames = options.GeneralOptions.MaxFrames; + this.skipMetadata = options.GeneralOptions.SkipMetadata; this.memoryAllocator = this.configuration.MemoryAllocator; + this.pngCrcChunkHandling = options.PngCrcChunkHandling; } - internal PngDecoderCore(DecoderOptions options, bool colorMetadataOnly) + internal PngDecoderCore(PngDecoderOptions options, bool colorMetadataOnly) { - this.Options = options; + this.Options = options.GeneralOptions; this.colorMetadataOnly = colorMetadataOnly; - this.maxFrames = options.MaxFrames; + this.maxFrames = options.GeneralOptions.MaxFrames; this.skipMetadata = true; - this.configuration = options.Configuration; + this.configuration = options.GeneralOptions.Configuration; this.memoryAllocator = this.configuration.MemoryAllocator; + this.pngCrcChunkHandling = options.PngCrcChunkHandling; } /// @@ -576,11 +583,23 @@ private static void ReadGammaChunk(PngMetadata pngMetadata, ReadOnlySpan d private void InitializeImage(ImageMetadata metadata, FrameControl frameControl, out Image image) where TPixel : unmanaged, IPixel { - image = Image.CreateUninitialized( - this.configuration, - this.header.Width, - this.header.Height, - metadata); + // When ignoring data CRCs, we can't use the image constructor that leaves the buffer uncleared. + if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll) + { + image = new Image( + this.configuration, + this.header.Width, + this.header.Height, + metadata); + } + else + { + image = Image.CreateUninitialized( + this.configuration, + this.header.Width, + this.header.Height, + metadata); + } PngFrameMetadata frameMetadata = image.Frames.RootFrame.Metadata.GetPngMetadata(); frameMetadata.FromChunk(in frameControl); @@ -803,6 +822,11 @@ private void DecodePixelData( break; default: + if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll) + { + goto EXIT; + } + PngThrowHelper.ThrowUnknownFilter(); break; } @@ -812,6 +836,7 @@ private void DecodePixelData( currentRow++; } + EXIT: blendMemory?.Dispose(); } @@ -903,6 +928,11 @@ private void DecodeInterlacedPixelData( break; default: + if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll) + { + goto EXIT; + } + PngThrowHelper.ThrowUnknownFilter(); break; } @@ -937,6 +967,7 @@ private void DecodeInterlacedPixelData( } } + EXIT: blendMemory?.Dispose(); } @@ -1364,7 +1395,7 @@ private void ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata met ReadOnlySpan compressedData = data[(zeroIndex + 2)..]; - if (this.TryUncompressTextData(compressedData, PngConstants.Encoding, out string? uncompressed) + if (this.TryDecompressTextData(compressedData, PngConstants.Encoding, out string? uncompressed) && !TryReadTextChunkMetadata(baseMetadata, name, uncompressed)) { metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty)); @@ -1508,19 +1539,19 @@ private void ReadColorProfileChunk(ImageMetadata metadata, ReadOnlySpan da ReadOnlySpan compressedData = data[(zeroIndex + 2)..]; - if (this.TryUncompressZlibData(compressedData, out byte[] iccpProfileBytes)) + if (this.TryDecompressZlibData(compressedData, out byte[] iccpProfileBytes)) { metadata.IccProfile = new IccProfile(iccpProfileBytes); } } /// - /// Tries to un-compress zlib compressed data. + /// Tries to decompress zlib compressed data. /// /// The compressed data. /// The uncompressed bytes array. /// True, if de-compressing was successful. - private unsafe bool TryUncompressZlibData(ReadOnlySpan compressedData, out byte[] uncompressedBytesArray) + private unsafe bool TryDecompressZlibData(ReadOnlySpan compressedData, out byte[] uncompressedBytesArray) { fixed (byte* compressedDataBase = compressedData) { @@ -1657,7 +1688,7 @@ private void ReadInternationalTextChunk(ImageMetadata metadata, ReadOnlySpan compressedData = data[dataStartIdx..]; - if (this.TryUncompressTextData(compressedData, PngConstants.TranslatedEncoding, out string? uncompressed)) + if (this.TryDecompressTextData(compressedData, PngConstants.TranslatedEncoding, out string? uncompressed)) { pngMetadata.TextData.Add(new PngTextData(keyword, uncompressed, language, translatedKeyword)); } @@ -1680,9 +1711,9 @@ private void ReadInternationalTextChunk(ImageMetadata metadata, ReadOnlySpanThe string encoding to use. /// The uncompressed value. /// The . - private bool TryUncompressTextData(ReadOnlySpan compressedData, Encoding encoding, [NotNullWhen(true)] out string? value) + private bool TryDecompressTextData(ReadOnlySpan compressedData, Encoding encoding, [NotNullWhen(true)] out string? value) { - if (this.TryUncompressZlibData(compressedData, out byte[] uncompressedData)) + if (this.TryDecompressZlibData(compressedData, out byte[] uncompressedData)) { value = encoding.GetString(uncompressedData); return true; @@ -1705,7 +1736,11 @@ private int ReadNextDataChunk() Span buffer = stackalloc byte[20]; - _ = this.currentStream.Read(buffer, 0, 4); + int length = this.currentStream.Read(buffer, 0, 4); + if (length == 0) + { + return 0; + } if (this.TryReadChunk(buffer, out PngChunk chunk)) { @@ -1734,7 +1769,11 @@ private int ReadNextFrameDataChunk() Span buffer = stackalloc byte[20]; - _ = this.currentStream.Read(buffer, 0, 4); + int length = this.currentStream.Read(buffer, 0, 4); + if (length == 0) + { + return 0; + } if (this.TryReadChunk(buffer, out PngChunk chunk)) { @@ -1771,21 +1810,27 @@ private bool TryReadChunk(Span buffer, out PngChunk chunk) return true; } - if (!this.TryReadChunkLength(buffer, out int length)) + if (this.currentStream.Position >= this.currentStream.Length - 1) { + // IEND chunk = default; + return false; + } + if (!this.TryReadChunkLength(buffer, out int length)) + { // IEND + chunk = default; return false; } - while (length < 0 || length > (this.currentStream.Length - this.currentStream.Position)) + while (length < 0) { // Not a valid chunk so try again until we reach a known chunk. if (!this.TryReadChunkLength(buffer, out length)) { + // IEND chunk = default; - return false; } } @@ -1797,13 +1842,14 @@ private bool TryReadChunk(Span buffer, out PngChunk chunk) if (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency && type != PngChunkType.Palette) { chunk = new PngChunk(length, type); - return true; } - long pos = this.currentStream.Position; + // A chunk might report a length that exceeds the length of the stream. + // Take the minimum of the two values to ensure we don't read past the end of the stream. + long position = this.currentStream.Position; chunk = new PngChunk( - length: length, + length: (int)Math.Min(length, this.currentStream.Length - position), type: type, data: this.ReadChunkData(length)); @@ -1813,7 +1859,7 @@ private bool TryReadChunk(Span buffer, out PngChunk chunk) // was only read to verifying the CRC is correct. if (type is PngChunkType.Data or PngChunkType.FrameData) { - this.currentStream.Position = pos; + this.currentStream.Position = position; } return true; @@ -1827,8 +1873,7 @@ private bool TryReadChunk(Span buffer, out PngChunk chunk) private void ValidateChunk(in PngChunk chunk, Span buffer) { uint inputCrc = this.ReadChunkCrc(buffer); - - if (chunk.IsCritical) + if (chunk.IsCritical(this.pngCrcChunkHandling)) { Span chunkType = stackalloc byte[4]; BinaryPrimitives.WriteUInt32BigEndian(chunkType, (uint)chunk.Type); diff --git a/src/ImageSharp/Formats/Png/PngDecoderOptions.cs b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs new file mode 100644 index 0000000000..ab6ba4770e --- /dev/null +++ b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs @@ -0,0 +1,18 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +namespace SixLabors.ImageSharp.Formats.Png; + +/// +/// Configuration options for decoding png images. +/// +public sealed class PngDecoderOptions : ISpecializedDecoderOptions +{ + /// + public DecoderOptions GeneralOptions { get; init; } = new DecoderOptions(); + + /// + /// Gets a value indicating how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG. + /// + public PngCrcChunkHandling PngCrcChunkHandling { get; init; } = PngCrcChunkHandling.IgnoreNonCritical; +} diff --git a/tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs b/tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs index 74c7fc1d09..324bd4783a 100644 --- a/tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs +++ b/tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs @@ -43,7 +43,7 @@ public void IfAutoLoadWellKnownFormatsIsTrueAllFormatsAreLoaded() Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); - Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); + Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 85f7bf7495..bc277bf485 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -470,6 +470,23 @@ public void Decode_InvalidDataChunkCrc_ThrowsException(TestImageProvider Assert.Contains("CRC Error. PNG IDAT chunk is corrupt!", ex.Message); } + // https://github.com/SixLabors/ImageSharp/pull/2589 + [Theory] + [WithFile(TestImages.Png.Bad.WrongCrcDataChunk, PixelTypes.Rgba32, true)] + [WithFile(TestImages.Png.Bad.Issue2589, PixelTypes.Rgba32, false)] + public void Decode_InvalidDataChunkCrc_IgnoreCrcErrors(TestImageProvider provider, bool compare) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder.Instance, new PngDecoderOptions() { PngCrcChunkHandling = PngCrcChunkHandling.IgnoreData }); + + image.DebugSave(provider); + if (compare) + { + // Magick cannot actually decode this image to compare. + image.CompareToOriginal(provider, new MagickReferenceDecoder(false)); + } + } + // https://github.com/SixLabors/ImageSharp/issues/1014 [Theory] [WithFileCollection(nameof(TestImagesIssue1014), PixelTypes.Rgba32)] @@ -640,9 +657,12 @@ static void RunTest(string providerDump, string nonContiguousBuffersStr) [Fact] public void Binary_PrematureEof() { - using EofHitCounter eofHitCounter = EofHitCounter.RunDecoder(TestImages.Png.Bad.FlagOfGermany0000016446); + PngDecoder decoder = PngDecoder.Instance; + PngDecoderOptions options = new() { PngCrcChunkHandling = PngCrcChunkHandling.IgnoreData }; + using EofHitCounter eofHitCounter = EofHitCounter.RunDecoder(TestImages.Png.Bad.FlagOfGermany0000016446, decoder, options); - Assert.True(eofHitCounter.EofHitCount <= 2); + // TODO: Try to reduce this to 1. + Assert.True(eofHitCounter.EofHitCount <= 3); Assert.Equal(new Size(200, 120), eofHitCounter.Image.Size); } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 943814ecf0..b6ea20f15c 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -157,6 +157,7 @@ public static class Bad public const string MissingPaletteChunk1 = "Png/missing_plte.png"; public const string MissingPaletteChunk2 = "Png/missing_plte_2.png"; public const string InvalidGammaChunk = "Png/length_gama.png"; + public const string Issue2589 = "Png/issues/Issue_2589.png"; // Zlib errors. public const string ZlibOverflow = "Png/zlib-overflow.png"; diff --git a/tests/ImageSharp.Tests/TestUtilities/EofHitCounter.cs b/tests/ImageSharp.Tests/TestUtilities/EofHitCounter.cs index d949ce0f2d..c5ffdd6489 100644 --- a/tests/ImageSharp.Tests/TestUtilities/EofHitCounter.cs +++ b/tests/ImageSharp.Tests/TestUtilities/EofHitCounter.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.IO; namespace SixLabors.ImageSharp.Tests.TestUtilities; @@ -21,6 +22,11 @@ public EofHitCounter(BufferedReadStream stream, Image image) public static EofHitCounter RunDecoder(string testImage) => RunDecoder(TestFile.Create(testImage).Bytes); + public static EofHitCounter RunDecoder(string testImage, T decoder, TO options) + where T : SpecializedImageDecoder + where TO : ISpecializedDecoderOptions + => RunDecoder(TestFile.Create(testImage).Bytes, decoder, options); + public static EofHitCounter RunDecoder(byte[] imageData) { BufferedReadStream stream = new(Configuration.Default, new MemoryStream(imageData)); @@ -28,6 +34,15 @@ public static EofHitCounter RunDecoder(byte[] imageData) return new EofHitCounter(stream, image); } + public static EofHitCounter RunDecoder(byte[] imageData, T decoder, TO options) + where T : SpecializedImageDecoder + where TO : ISpecializedDecoderOptions + { + BufferedReadStream stream = new(options.GeneralOptions.Configuration, new MemoryStream(imageData)); + Image image = decoder.Decode(options, stream); + return new EofHitCounter(stream, image); + } + public void Dispose() { this.stream.Dispose(); diff --git a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs index ae09c4f3f2..80b5536ebd 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs @@ -31,12 +31,17 @@ protected override Image Decode(DecoderOptions options, Stream s { IgnoreFileSize = !this.validate }; + PngReadDefines pngReadDefines = new() + { + IgnoreCrc = !this.validate + }; MagickReadSettings settings = new() { FrameCount = (int)options.MaxFrames }; settings.SetDefines(bmpReadDefines); + settings.SetDefines(pngReadDefines); using MagickImageCollection magickImageCollection = new(stream, settings); List> framesList = new(); diff --git a/tests/Images/Input/Png/issues/Issue_2589.png b/tests/Images/Input/Png/issues/Issue_2589.png new file mode 100644 index 0000000000..f2f159ea70 --- /dev/null +++ b/tests/Images/Input/Png/issues/Issue_2589.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:f7e87701298da4ba0a5cc14e9c04d810125f4958aa338255b14fd19dec15b677 +size 62662