Browse Source

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.
pull/3137/head
James Jackson-South 3 weeks ago
parent
commit
dfdc0734e3
  1. 6
      src/ImageSharp/Compression/Zlib/ZlibInflateReader.cs
  2. 2
      src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs
  3. 23
      src/ImageSharp/Formats/Png/PngDecoderCore.cs
  4. 2
      src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs
  5. 55
      tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs

6
src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs → src/ImageSharp/Compression/Zlib/ZlibInflateReader.cs

@ -12,7 +12,7 @@ namespace SixLabors.ImageSharp.Compression.Zlib;
/// <see cref="DeflateStream"/> over the remaining DEFLATE payload. The /// <see cref="DeflateStream"/> over the remaining DEFLATE payload. The
/// Adler-32 trailer is not validated. /// Adler-32 trailer is not validated.
/// </summary> /// </summary>
internal sealed class ZlibInflateStream : IDisposable internal sealed class ZlibInflateReader : IDisposable
{ {
/// <summary> /// <summary>
/// Used to read the Adler-32 and Crc-32 checksums. /// Used to read the Adler-32 and Crc-32 checksums.
@ -23,10 +23,10 @@ internal sealed class ZlibInflateStream : IDisposable
private readonly ChunkedReadStream segmentStream; private readonly ChunkedReadStream segmentStream;
public ZlibInflateStream(BufferedReadStream innerStream) public ZlibInflateReader(BufferedReadStream innerStream)
=> this.segmentStream = new ChunkedReadStream(innerStream); => this.segmentStream = new ChunkedReadStream(innerStream);
public ZlibInflateStream(BufferedReadStream innerStream, Func<int> getData) public ZlibInflateReader(BufferedReadStream innerStream, Func<int> getData)
=> this.segmentStream = new ChunkedReadStream(innerStream, getData); => this.segmentStream = new ChunkedReadStream(innerStream, getData);
/// <summary> /// <summary>

2
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<byte> uncompressed, uint uncompressedBytes) protected static int UndoZipCompression(BufferedReadStream stream, uint compressedBytes, Span<byte> uncompressed, uint uncompressedBytes)
{ {
long pos = stream.Position; long pos = stream.Position;
using ZlibInflateStream inflateStream = new( using ZlibInflateReader inflateStream = new(
stream, stream,
() => () =>
{ {

23
src/ImageSharp/Formats/Png/PngDecoderCore.cs

@ -321,7 +321,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore
case PngChunkType.End: case PngChunkType.End:
goto EOF; goto EOF;
case PngChunkType.ProprietaryApple: case PngChunkType.ProprietaryApple:
this.isCgbi = true; this.ReadCgbiChunk();
break; break;
} }
} }
@ -525,7 +525,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore
goto EOF; goto EOF;
case PngChunkType.ProprietaryApple: case PngChunkType.ProprietaryApple:
this.isCgbi = true; this.ReadCgbiChunk();
break; break;
default: default:
@ -788,7 +788,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore
return; return;
} }
using ZlibInflateStream inflateStream = new(this.currentStream, getData); using ZlibInflateReader inflateStream = new(this.currentStream, getData);
if (!inflateStream.AllocateNewBytes(chunkLength, !this.hasImageData)) if (!inflateStream.AllocateNewBytes(chunkLength, !this.hasImageData))
{ {
return; return;
@ -1458,6 +1458,21 @@ internal sealed class PngDecoderCore : ImageDecoderCore
} }
} }
/// <summary>
/// Marks the image as CgBI and validates a header that has already been read.
/// </summary>
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) private static void ThrowIfInvalidCgbiContent(in PngHeader header)
{ {
if (header.BitDepth != 8 || (header.ColorType is not PngColorType.Rgb and not PngColorType.RgbWithAlpha)) 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 MemoryStream memoryStreamOutput = new(compressedData.Length);
using UnmanagedMemoryStream memoryStreamInput = new(compressedDataBase, compressedData.Length); using UnmanagedMemoryStream memoryStreamInput = new(compressedDataBase, compressedData.Length);
using BufferedReadStream bufferedStream = new(this.configuration, memoryStreamInput); using BufferedReadStream bufferedStream = new(this.configuration, memoryStreamInput);
using ZlibInflateStream inflateStream = new(bufferedStream); using ZlibInflateReader inflateStream = new(bufferedStream);
Span<byte> destUncompressedData = destBuffer.GetSpan(); Span<byte> destUncompressedData = destBuffer.GetSpan();
if (!inflateStream.AllocateNewBytes(compressedData.Length, false)) if (!inflateStream.AllocateNewBytes(compressedData.Length, false))

2
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<byte> buffer, CancellationToken cancellationToken) protected override void Decompress(BufferedReadStream stream, int byteCount, int stripHeight, Span<byte> buffer, CancellationToken cancellationToken)
{ {
long pos = stream.Position; long pos = stream.Position;
using (ZlibInflateStream deframeStream = new( using (ZlibInflateReader deframeStream = new(
stream, stream,
() => () =>
{ {

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

@ -1,6 +1,7 @@
// Copyright (c) Six Labors. // Copyright (c) Six Labors.
// Licensed under the Six Labors Split License. // Licensed under the Six Labors Split License.
using System.Buffers.Binary;
using System.Runtime.Intrinsics.X86; using System.Runtime.Intrinsics.X86;
using Microsoft.DotNet.RemoteExecutor; using Microsoft.DotNet.RemoteExecutor;
using SixLabors.ImageSharp.Formats; 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); 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<InvalidImageContentException>(() => 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<InvalidImageContentException>(
() => { using Image<Rgba32> image = PngDecoder.Instance.Decode<Rgba32>(DecoderOptions.Default, stream); });
Assert.Contains("CgBI is only supported for 8-bit truecolor images", ex.Message);
}
/// <summary>
/// Moves the first PNG chunk after the second while preserving each chunk's data and CRC.
/// </summary>
/// <param name="source">A PNG whose first two chunks should be exchanged.</param>
/// <returns>A copy of the PNG with its first two chunks exchanged.</returns>
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] [Theory]
[WithFile(TestImages.Png.Splash, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Splash, PixelTypes.Rgba32)]
[WithFile(TestImages.Png.Bike, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Bike, PixelTypes.Rgba32)]

Loading…
Cancel
Save