From dfdc0734e34899695c170db89ebc2d522dac455c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 23 Jul 2026 19:10:07 +1000 Subject: [PATCH] Validate CgBI chunk order in PNG decoder Handle CgBI chunks consistently when they appear after IHDR by routing both identify/decode paths through a new `ReadCgbiChunk()` method that immediately validates already-read headers. This prevents invalid CgBI images from bypassing compatibility checks when chunk order is unusual. Added PNG tests that reorder the first two chunks to confirm incompatible CgBI headers now throw in both identify and decode flows. Also renames `ZlibInflateStream` to `ZlibInflateReader` and updates EXR/PNG/TIFF call sites for clearer intent. --- ...bInflateStream.cs => ZlibInflateReader.cs} | 6 +- .../Exr/Compression/ExrBaseDecompressor.cs | 2 +- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 23 ++++++-- .../Decompressors/DeflateTiffCompression.cs | 2 +- .../Formats/Png/PngDecoderTests.cs | 55 +++++++++++++++++++ 5 files changed, 79 insertions(+), 9 deletions(-) rename src/ImageSharp/Compression/Zlib/{ZlibInflateStream.cs => ZlibInflateReader.cs} (95%) diff --git a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Compression/Zlib/ZlibInflateReader.cs similarity index 95% rename from src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs rename to src/ImageSharp/Compression/Zlib/ZlibInflateReader.cs index 11f34dac8..de69717da 100644 --- a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Compression/Zlib/ZlibInflateReader.cs @@ -12,7 +12,7 @@ namespace SixLabors.ImageSharp.Compression.Zlib; /// over the remaining DEFLATE payload. The /// Adler-32 trailer is not validated. /// -internal sealed class ZlibInflateStream : IDisposable +internal sealed class ZlibInflateReader : IDisposable { /// /// Used to read the Adler-32 and Crc-32 checksums. @@ -23,10 +23,10 @@ internal sealed class ZlibInflateStream : IDisposable private readonly ChunkedReadStream segmentStream; - public ZlibInflateStream(BufferedReadStream innerStream) + public ZlibInflateReader(BufferedReadStream innerStream) => this.segmentStream = new ChunkedReadStream(innerStream); - public ZlibInflateStream(BufferedReadStream innerStream, Func getData) + public ZlibInflateReader(BufferedReadStream innerStream, Func getData) => this.segmentStream = new ChunkedReadStream(innerStream, getData); /// diff --git a/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs b/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs index 1b2d95ba5..f598955f4 100644 --- a/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs +++ b/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs @@ -45,7 +45,7 @@ internal abstract class ExrBaseDecompressor : ExrBaseCompression protected static int UndoZipCompression(BufferedReadStream stream, uint compressedBytes, Span uncompressed, uint uncompressedBytes) { long pos = stream.Position; - using ZlibInflateStream inflateStream = new( + using ZlibInflateReader inflateStream = new( stream, () => { diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index f3e2bbdbe..28f9a989b 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -321,7 +321,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore case PngChunkType.End: goto EOF; case PngChunkType.ProprietaryApple: - this.isCgbi = true; + this.ReadCgbiChunk(); break; } } @@ -525,7 +525,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore goto EOF; case PngChunkType.ProprietaryApple: - this.isCgbi = true; + this.ReadCgbiChunk(); break; default: @@ -788,7 +788,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore return; } - using ZlibInflateStream inflateStream = new(this.currentStream, getData); + using ZlibInflateReader inflateStream = new(this.currentStream, getData); if (!inflateStream.AllocateNewBytes(chunkLength, !this.hasImageData)) { return; @@ -1458,6 +1458,21 @@ internal sealed class PngDecoderCore : ImageDecoderCore } } + /// + /// Marks the image as CgBI and validates a header that has already been read. + /// + private void ReadCgbiChunk() + { + this.isCgbi = true; + + // Although Apple's pngcrush normally writes CgBI before IHDR, accepting the + // reverse order must apply the same compatibility validation. + if (!Equals(this.header, default(PngHeader))) + { + ThrowIfInvalidCgbiContent(this.header); + } + } + private static void ThrowIfInvalidCgbiContent(in PngHeader header) { if (header.BitDepth != 8 || (header.ColorType is not PngColorType.Rgb and not PngColorType.RgbWithAlpha)) @@ -1952,7 +1967,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore using MemoryStream memoryStreamOutput = new(compressedData.Length); using UnmanagedMemoryStream memoryStreamInput = new(compressedDataBase, compressedData.Length); using BufferedReadStream bufferedStream = new(this.configuration, memoryStreamInput); - using ZlibInflateStream inflateStream = new(bufferedStream); + using ZlibInflateReader inflateStream = new(bufferedStream); Span destUncompressedData = destBuffer.GetSpan(); if (!inflateStream.AllocateNewBytes(compressedData.Length, false)) diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs index 4e176f28d..d3b65c537 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs @@ -54,7 +54,7 @@ internal sealed class DeflateTiffCompression : TiffBaseDecompressor protected override void Decompress(BufferedReadStream stream, int byteCount, int stripHeight, Span buffer, CancellationToken cancellationToken) { long pos = stream.Position; - using (ZlibInflateStream deframeStream = new( + using (ZlibInflateReader deframeStream = new( stream, () => { diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 2fbbe695e..88ed512d1 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Buffers.Binary; using System.Runtime.Intrinsics.X86; using Microsoft.DotNet.RemoteExecutor; using SixLabors.ImageSharp.Formats; @@ -789,6 +790,60 @@ public partial class PngDecoderTests Assert.Contains("CgBI is only supported for 8-bit truecolor images", ex.Message); } + [Theory] + [InlineData(TestImages.Png.Cgbi.BitDepth16)] + [InlineData(TestImages.Png.Cgbi.Palette)] + public void Identify_CgBI_AfterHeader_IncompatibleHeader_ThrowsInvalidImageContentException(string imagePath) + { + TestFile testFile = TestFile.Create(imagePath); + byte[] reordered = MoveFirstPngChunkAfterSecond(testFile.Bytes); + using MemoryStream stream = new(reordered, false); + + InvalidImageContentException ex = Assert.Throws(() => Image.Identify(stream)); + Assert.Contains("CgBI is only supported for 8-bit truecolor images", ex.Message); + } + + [Theory] + [InlineData(TestImages.Png.Cgbi.BitDepth16)] + [InlineData(TestImages.Png.Cgbi.Palette)] + public void Decode_CgBI_AfterHeader_IncompatibleHeader_ThrowsInvalidImageContentException(string imagePath) + { + TestFile testFile = TestFile.Create(imagePath); + byte[] reordered = MoveFirstPngChunkAfterSecond(testFile.Bytes); + using MemoryStream stream = new(reordered, false); + + InvalidImageContentException ex = Assert.Throws( + () => { using Image image = PngDecoder.Instance.Decode(DecoderOptions.Default, stream); }); + Assert.Contains("CgBI is only supported for 8-bit truecolor images", ex.Message); + } + + /// + /// Moves the first PNG chunk after the second while preserving each chunk's data and CRC. + /// + /// A PNG whose first two chunks should be exchanged. + /// A copy of the PNG with its first two chunks exchanged. + private static byte[] MoveFirstPngChunkAfterSecond(byte[] source) + { + const int signatureLength = 8; + const int chunkOverheadLength = 12; + + int firstChunkLength = BinaryPrimitives.ReadInt32BigEndian(source.AsSpan(signatureLength, 4)) + chunkOverheadLength; + int secondChunkOffset = signatureLength + firstChunkLength; + int secondChunkLength = BinaryPrimitives.ReadInt32BigEndian(source.AsSpan(secondChunkOffset, 4)) + chunkOverheadLength; + byte[] reordered = new byte[source.Length]; + + source.AsSpan(0, signatureLength).CopyTo(reordered); + source.AsSpan(secondChunkOffset, secondChunkLength).CopyTo(reordered.AsSpan(signatureLength)); + source.AsSpan(signatureLength, firstChunkLength).CopyTo(reordered.AsSpan(signatureLength + secondChunkLength)); + + // Chunk CRCs cover only each chunk's type and data, so moving complete chunks + // leaves both checksums valid and isolates ordering as the tested behavior. + source.AsSpan(secondChunkOffset + secondChunkLength) + .CopyTo(reordered.AsSpan(signatureLength + secondChunkLength + firstChunkLength)); + + return reordered; + } + [Theory] [WithFile(TestImages.Png.Splash, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Bike, PixelTypes.Rgba32)]