Merge pull request #2589 from svenclaesson/better_handle_corrupt_png

Relaxed handle of corrupt png files

src/ImageSharp/Formats/Png/PngChunk.cs

@@ -41,9 +41,13 @@ public PngChunk(int length, PngChunkType type, IMemoryOwner<byte> data = null)
     /// <summary>
     /// Gets a value indicating whether the given chunk is critical to decoding
     /// </summary>
-    public bool IsCritical =>
-        this.Type is PngChunkType.Header or
-        PngChunkType.Palette or
-        PngChunkType.Data or
-        PngChunkType.FrameData;
+    /// <param name="handling">The chunk CRC handling behavior.</param>
+    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,
+        };
 }

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;
+
+/// <summary>
+/// Specifies how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG.
+/// </summary>
+public enum PngCrcChunkHandling
+{
+    /// <summary>
+    /// Do not ignore any CRC chunk errors.
+    /// </summary>
+    IgnoreNone,
+
+    /// <summary>
+    /// Ignore CRC errors in non critical chunks.
+    /// </summary>
+    IgnoreNonCritical,
+
+    /// <summary>
+    /// Ignore CRC errors in data chunks.
+    /// </summary>
+    IgnoreData,
+
+    /// <summary>
+    /// Ignore CRC errors in all chunks.
+    /// </summary>
+    IgnoreAll
+}

src/ImageSharp/Formats/Png/PngDecoder.cs

@@ -8,7 +8,7 @@ namespace SixLabors.ImageSharp.Formats.Png;
 /// <summary>
 /// Decoder for generating an image out of a png encoded stream.
 /// </summary>
-public sealed class PngDecoder : ImageDecoder
+public sealed class PngDecoder : SpecializedImageDecoder<PngDecoderOptions>
 {
     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);
     }
 
     /// <inheritdoc/>
-    protected override Image<TPixel> Decode<TPixel>(DecoderOptions options, Stream stream, CancellationToken cancellationToken)
+    protected override Image<TPixel> Decode<TPixel>(PngDecoderOptions options, Stream stream, CancellationToken cancellationToken)
     {
         Guard.NotNull(options, nameof(options));
         Guard.NotNull(stream, nameof(stream));
 
         PngDecoderCore decoder = new(options);
-        Image<TPixel> image = decoder.Decode<TPixel>(options.Configuration, stream, cancellationToken);
+        Image<TPixel> image = decoder.Decode<TPixel>(options.GeneralOptions.Configuration, stream, cancellationToken);
 
-        ScaleToTargetSize(options, image);
+        ScaleToTargetSize(options.GeneralOptions, image);
 
         return image;
     }
 
     /// <inheritdoc/>
-    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<Rgba32>(options, stream, cancellationToken);
         }
     }
+
+    /// <inheritdoc/>
+    protected override PngDecoderOptions CreateDefaultSpecializedOptions(DecoderOptions options) => new PngDecoderOptions() { GeneralOptions = options };
 }

src/ImageSharp/Formats/Png/PngDecoderCore.cs

@@ -114,27 +114,34 @@ internal sealed class PngDecoderCore : IImageDecoderInternals
     /// </summary>
     private PngChunk? nextChunk;
 
+    /// <summary>
+    /// How to handle CRC errors.
+    /// </summary>
+    private readonly PngCrcChunkHandling pngCrcChunkHandling;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="PngDecoderCore"/> class.
     /// </summary>
     /// <param name="options">The decoder options.</param>
-    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;
     }
 
     /// <inheritdoc/>
@@ -576,11 +583,23 @@ private static void ReadGammaChunk(PngMetadata pngMetadata, ReadOnlySpan<byte> d
     private void InitializeImage<TPixel>(ImageMetadata metadata, FrameControl frameControl, out Image<TPixel> image)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        image = Image.CreateUninitialized<TPixel>(
-            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<TPixel>(
+                this.configuration,
+                this.header.Width,
+                this.header.Height,
+                metadata);
+        }
+        else
+        {
+            image = Image.CreateUninitialized<TPixel>(
+                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<TPixel>(
                     break;
 
                 default:
+                    if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll)
+                    {
+                        goto EXIT;
+                    }
+
                     PngThrowHelper.ThrowUnknownFilter();
                     break;
             }
@@ -812,6 +836,7 @@ private void DecodePixelData<TPixel>(
             currentRow++;
         }
 
+        EXIT:
         blendMemory?.Dispose();
     }
 
@@ -903,6 +928,11 @@ private void DecodeInterlacedPixelData<TPixel>(
                         break;
 
                     default:
+                        if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll)
+                        {
+                            goto EXIT;
+                        }
+
                         PngThrowHelper.ThrowUnknownFilter();
                         break;
                 }
@@ -937,6 +967,7 @@ private void DecodeInterlacedPixelData<TPixel>(
             }
         }
 
+        EXIT:
         blendMemory?.Dispose();
     }
 
@@ -1364,7 +1395,7 @@ private void ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata met
 
         ReadOnlySpan<byte> 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<byte> da
 
         ReadOnlySpan<byte> compressedData = data[(zeroIndex + 2)..];
 
-        if (this.TryUncompressZlibData(compressedData, out byte[] iccpProfileBytes))
+        if (this.TryDecompressZlibData(compressedData, out byte[] iccpProfileBytes))
         {
             metadata.IccProfile = new IccProfile(iccpProfileBytes);
         }
     }
 
     /// <summary>
-    /// Tries to un-compress zlib compressed data.
+    /// Tries to decompress zlib compressed data.
     /// </summary>
     /// <param name="compressedData">The compressed data.</param>
     /// <param name="uncompressedBytesArray">The uncompressed bytes array.</param>
     /// <returns>True, if de-compressing was successful.</returns>
-    private unsafe bool TryUncompressZlibData(ReadOnlySpan<byte> compressedData, out byte[] uncompressedBytesArray)
+    private unsafe bool TryDecompressZlibData(ReadOnlySpan<byte> compressedData, out byte[] uncompressedBytesArray)
     {
         fixed (byte* compressedDataBase = compressedData)
         {
@@ -1657,7 +1688,7 @@ private void ReadInternationalTextChunk(ImageMetadata metadata, ReadOnlySpan<byt
         {
             ReadOnlySpan<byte> 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, ReadOnlySpan<byt
     /// <param name="encoding">The string encoding to use.</param>
     /// <param name="value">The uncompressed value.</param>
     /// <returns>The <see cref="bool"/>.</returns>
-    private bool TryUncompressTextData(ReadOnlySpan<byte> compressedData, Encoding encoding, [NotNullWhen(true)] out string? value)
+    private bool TryDecompressTextData(ReadOnlySpan<byte> 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<byte> 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<byte> 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<byte> 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<byte> 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<byte> 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<byte> buffer, out PngChunk chunk)
     private void ValidateChunk(in PngChunk chunk, Span<byte> buffer)
     {
         uint inputCrc = this.ReadChunkCrc(buffer);
-
-        if (chunk.IsCritical)
+        if (chunk.IsCritical(this.pngCrcChunkHandling))
         {
             Span<byte> chunkType = stackalloc byte[4];
             BinaryPrimitives.WriteUInt32BigEndian(chunkType, (uint)chunk.Type);

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;
+
+/// <summary>
+/// Configuration options for decoding png images.
+/// </summary>
+public sealed class PngDecoderOptions : ISpecializedDecoderOptions
+{
+    /// <inheritdoc/>
+    public DecoderOptions GeneralOptions { get; init; } = new DecoderOptions();
+
+    /// <summary>
+    /// Gets a value indicating how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG.
+    /// </summary>
+    public PngCrcChunkHandling PngCrcChunkHandling { get; init; } = PngCrcChunkHandling.IgnoreNonCritical;
+}

tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs

@@ -43,7 +43,7 @@ public void IfAutoLoadWellKnownFormatsIsTrueAllFormatsAreLoaded()
         Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<PngDecoder>().Count());
         Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<BmpDecoder>().Count());
         Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<JpegDecoder>().Count());
-        Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<BmpDecoder>().Count());
+        Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<GifDecoder>().Count());
         Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<TgaDecoder>().Count());
         Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<TiffDecoder>().Count());
         Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType<WebpDecoder>().Count());