From 3c78bec7edbb8f2fd412b4558b87bb4b12e0abf8 Mon Sep 17 00:00:00 2001 From: Jason Finch Date: Sun, 18 Oct 2026 09:11:13 +1000 Subject: [PATCH] Decode PNG data split by empty chunks, reject short palettes and use big-endian 16-bit tRNS Zero-length IDAT and fdAT chunks are valid, but the data chunk readers returned their length of 0, which the inflater treats as the end of the compressed stream. An empty chunk inside the zlib header, or two in a row, broke decoding. The readers now skip empty chunks. A PLTE chunk shorter than one entry produced an empty color table, which the palette scanline processor indexed outside of. It is now treated as a missing palette, so decoding throws InvalidImageContentException. 16-bit grayscale and truecolor tRNS samples are big-endian, like all PNG multi-byte values, but were read and written little-endian. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 78 +++-- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 9 +- .../Formats/Png/PngDecoderTests.Chunks.cs | 298 ++++++++++++++++++ .../Formats/Png/PngEncoderTests.Chunks.cs | 44 +++ 4 files changed, 394 insertions(+), 35 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 7df41d31ea..bcd46d805f 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1338,7 +1338,9 @@ private void ProcessInterlacedDefilteredScanline( /// The png metadata. private static void AssignColorPalette(ReadOnlySpan palette, ReadOnlySpan alpha, PngMetadata pngMetadata) { - if (palette.Length == 0) + // A palette chunk too short to hold a single entry would produce an empty color table, which the + // scanline processors would index outside of. Treat it as missing so indexed decoding reports it. + if (palette.Length < Unsafe.SizeOf()) { return; } @@ -1380,9 +1382,11 @@ private void AssignTransparentMarkers(ReadOnlySpan alpha, PngMetadata pngM { if (this.header.BitDepth == 16) { - ushort rc = BinaryPrimitives.ReadUInt16LittleEndian(alpha[..2]); - ushort gc = BinaryPrimitives.ReadUInt16LittleEndian(alpha.Slice(2, 2)); - ushort bc = BinaryPrimitives.ReadUInt16LittleEndian(alpha.Slice(4, 2)); + // tRNS samples are stored big-endian like all other PNG multi-byte values, + // matching the byte order used for the 16-bit samples in the image data. + ushort rc = BinaryPrimitives.ReadUInt16BigEndian(alpha[..2]); + ushort gc = BinaryPrimitives.ReadUInt16BigEndian(alpha.Slice(2, 2)); + ushort bc = BinaryPrimitives.ReadUInt16BigEndian(alpha.Slice(4, 2)); pngMetadata.TransparentColor = Color.FromPixel(new Rgb48(rc, gc, bc)); return; @@ -1400,7 +1404,7 @@ private void AssignTransparentMarkers(ReadOnlySpan alpha, PngMetadata pngM { if (this.header.BitDepth == 16) { - pngMetadata.TransparentColor = Color.FromPixel(new L16(BinaryPrimitives.ReadUInt16LittleEndian(alpha[..2]))); + pngMetadata.TransparentColor = Color.FromPixel(new L16(BinaryPrimitives.ReadUInt16BigEndian(alpha[..2]))); } else { @@ -2183,24 +2187,30 @@ private int ReadNextDataChunk() Span buffer = stackalloc byte[20]; - int length = this.currentStream.Read(buffer, 0, 4); - if (length == 0) + // The compressed stream is the concatenation of the payloads of all consecutive data chunks, and the + // specification permits zero-length data chunks. A return value of 0 tells the inflater that the + // compressed stream has ended, so empty chunks are skipped rather than reported. + while (true) { - return 0; - } + // Skip the CRC of the previous chunk. + int length = this.currentStream.Read(buffer, 0, 4); + if (length == 0 || !this.TryReadChunk(buffer, out PngChunk chunk)) + { + return 0; + } - if (this.TryReadChunk(buffer, out PngChunk chunk)) - { - if (chunk.Type is PngChunkType.Data or PngChunkType.FrameData) + if (chunk.Type is not (PngChunkType.Data or PngChunkType.FrameData)) { - chunk.Data?.Dispose(); - return chunk.Length; + this.nextChunk = chunk; + return 0; } - this.nextChunk = chunk; + chunk.Data?.Dispose(); + if (chunk.Length > 0) + { + return chunk.Length; + } } - - return 0; } /// @@ -2216,26 +2226,32 @@ private int ReadNextFrameDataChunk() Span buffer = stackalloc byte[20]; - int length = this.currentStream.Read(buffer, 0, 4); - if (length == 0) - { - return 0; - } - - if (this.TryReadChunk(buffer, out PngChunk chunk)) + // As with data chunks, frame data chunks with no payload after the sequence number are valid and are + // skipped, so that a return value of 0 only ever means that the frame's compressed stream has ended. + while (true) { - if (chunk.Type is PngChunkType.FrameData) + // Skip the CRC of the previous chunk. + int length = this.currentStream.Read(buffer, 0, 4); + if (length == 0 || !this.TryReadChunk(buffer, out PngChunk chunk)) { - chunk.Data?.Dispose(); + return 0; + } - this.currentStream.Position += 4; // Skip sequence number - return chunk.Length - 4; + if (chunk.Type is not PngChunkType.FrameData) + { + this.nextChunk = chunk; + return 0; } - this.nextChunk = chunk; - } + chunk.Data?.Dispose(); - return 0; + this.currentStream.Position += 4; // Skip sequence number + int dataLength = chunk.Length - 4; + if (dataLength > 0) + { + return dataLength; + } + } } /// diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index caf2393d89..13171c8181 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -1247,10 +1247,11 @@ private void WriteTransparencyChunk(Stream stream, PngMetadata pngMetadata) { if (this.use16Bit) { + // tRNS samples are stored big-endian like all other PNG multi-byte values. Rgb48 rgb = pngMetadata.TransparentColor.Value.ToPixel(); - BinaryPrimitives.WriteUInt16LittleEndian(alpha, rgb.R); - BinaryPrimitives.WriteUInt16LittleEndian(alpha.Slice(2, 2), rgb.G); - BinaryPrimitives.WriteUInt16LittleEndian(alpha.Slice(4, 2), rgb.B); + BinaryPrimitives.WriteUInt16BigEndian(alpha, rgb.R); + BinaryPrimitives.WriteUInt16BigEndian(alpha.Slice(2, 2), rgb.G); + BinaryPrimitives.WriteUInt16BigEndian(alpha.Slice(4, 2), rgb.B); this.WriteChunk(stream, PngChunkType.Transparency, this.chunkDataBuffer, 0, 6); } @@ -1269,7 +1270,7 @@ private void WriteTransparencyChunk(Stream stream, PngMetadata pngMetadata) if (this.use16Bit) { L16 l16 = pngMetadata.TransparentColor.Value.ToPixel(); - BinaryPrimitives.WriteUInt16LittleEndian(alpha, l16.PackedValue); + BinaryPrimitives.WriteUInt16BigEndian(alpha, l16.PackedValue); this.WriteChunk(stream, PngChunkType.Transparency, this.chunkDataBuffer, 0, 2); } else diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs index 252ea49603..e99f2ae4be 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs @@ -2,11 +2,13 @@ // Licensed under the Six Labors Split License. using System.Buffers.Binary; +using System.IO.Compression; using System.IO.Hashing; using System.Text; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests.Formats.Png; @@ -291,6 +293,302 @@ public void Decode_tRnsChunk_WithAlphaLengthGreaterColorTableLength_ShouldNotThr using Image image = Image.Load(stream); } + /// + /// The compressed image data may be split across data chunks at any byte, including inside the zlib header, + /// and zero-length data chunks are permitted. Every layout must decode to the same pixels. + /// + /// The name of the data chunk layout to write. + [Theory] + [InlineData("LeadingEmpty")] + [InlineData("ConsecutiveLeadingEmpty")] + [InlineData("FirstOneByte")] + [InlineData("EmptyWithinHeader")] + [InlineData("EveryOneByte")] + [InlineData("EveryOneByteWithEmpty")] + [InlineData("EmptyBetween")] + [InlineData("TrailingEmpty")] + public void Decode_CompressedDataSplitAcrossDataChunks_DecodesPixelExact(string layout) + { + using Image expected = CreateNoiseImage(40, 30, 1); + using MemoryStream stream = new(); + expected.Save(stream, new PngEncoder { ColorType = PngColorType.RgbWithAlpha }); + List<(string Type, byte[] Data)> chunks = ReadChunks(stream.ToArray()); + + byte[] zlib = [.. chunks.Where(c => c.Type == "IDAT").SelectMany(c => c.Data)]; + byte[][] segments = layout switch + { + "LeadingEmpty" => [[], zlib], + "ConsecutiveLeadingEmpty" => [[], [], zlib], + "FirstOneByte" => [zlib[..1], zlib[1..]], + "EmptyWithinHeader" => [zlib[..1], [], zlib[1..]], + "EveryOneByte" => [.. zlib.Select(b => new[] { b })], + "EveryOneByteWithEmpty" => [.. zlib.SelectMany(b => new[] { [b], Array.Empty() })], + "EmptyBetween" => [zlib[..(zlib.Length / 2)], [], zlib[(zlib.Length / 2)..]], + "TrailingEmpty" => [zlib, []], + _ => throw new ArgumentOutOfRangeException(nameof(layout)), + }; + + // Replace the encoder's data chunks with the requested layout, keeping every other chunk in place. + using MemoryStream output = new(); + output.Write(stream.ToArray().AsSpan(0, 8)); + foreach ((string type, byte[] data) in chunks) + { + if (type == "IDAT") + { + continue; + } + + if (type == "IEND") + { + foreach (byte[] segment in segments) + { + WriteChunk(output, "IDAT", segment); + } + } + + WriteChunk(output, type, data); + } + + using Image actual = Image.Load(output.ToArray()); + + ImageComparer.Exact.VerifySimilarity(expected, actual); + } + + /// + /// A frame data chunk may carry nothing but its sequence number. Such chunks must not end the frame's + /// compressed data early, wherever they fall. + /// + [Fact] + public void Decode_EmptyFrameDataChunks_DecodesPixelExact() + { + using Image image = CreateNoiseImage(40, 30, 1); + using (Image frame = CreateNoiseImage(40, 30, 2)) + { + image.Frames.AddFrame(frame.Frames.RootFrame); + } + + using MemoryStream stream = new(); + image.Save(stream, new PngEncoder { ColorType = PngColorType.RgbWithAlpha }); + byte[] encoded = stream.ToArray(); + List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); + Assert.Contains(chunks, c => c.Type == "fdAT"); + + // Split every frame data chunk inside its zlib header, around an empty frame data chunk, and renumber + // the shared fcTL/fdAT sequence so the result stays a well-formed animation. + using MemoryStream output = new(); + output.Write(encoded.AsSpan(0, 8)); + uint sequenceNumber = 0; + foreach ((string type, byte[] data) in chunks) + { + if (type == "fdAT") + { + ReadOnlySpan payload = data.AsSpan(4); + WriteChunk(output, type, [.. GetSequenceNumber(sequenceNumber++), .. payload[..1]]); + WriteChunk(output, type, GetSequenceNumber(sequenceNumber++)); + WriteChunk(output, type, [.. GetSequenceNumber(sequenceNumber++), .. payload[1..]]); + continue; + } + + if (type == "fcTL") + { + BinaryPrimitives.WriteUInt32BigEndian(data, sequenceNumber++); + } + + WriteChunk(output, type, data); + } + + using Image expected = Image.Load(encoded); + using Image actual = Image.Load(output.ToArray()); + + Assert.Equal(expected.Frames.Count, actual.Frames.Count); + for (int i = 0; i < expected.Frames.Count; i++) + { + using Image expectedFrame = expected.Frames.CloneFrame(i); + using Image actualFrame = actual.Frames.CloneFrame(i); + ImageComparer.Exact.VerifySimilarity(expectedFrame, actualFrame); + } + } + + /// + /// A palette chunk too short to hold one entry leaves no colors to index, so it is reported as a missing palette + /// instead of reading outside the color table. + /// + /// The interlace mode of the image. + [Theory] + [InlineData(PngInterlaceMode.None)] + [InlineData(PngInterlaceMode.Adam7)] + public void Decode_PaletteChunkShorterThanOneEntry_ThrowsInvalidImageContentException(PngInterlaceMode interlaceMode) + { + // Zeroed scanlines: filter type None and palette index 0. A 9x9 image has 9 rows of 1 + 9 bytes, or + // 100 bytes across the seven Adam7 passes, each row of which also starts with a filter byte. + byte[] indices = interlaceMode == PngInterlaceMode.Adam7 ? new byte[100] : new byte[90]; + using MemoryStream stream = new(); + stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + WriteChunk(stream, "IHDR", CreateHeaderData(9, 9, 8, PngColorType.Palette, interlaceMode)); + WriteChunk(stream, "PLTE", [10, 20]); + WriteChunk(stream, "IDAT", Compress(indices)); + WriteChunk(stream, "IEND", []); + + Assert.Throws(() => Image.Load(stream.ToArray())); + } + + /// + /// 16-bit grayscale tRNS samples are big-endian, so 0x0102 is transparent and its byte-swapped value is not. + /// + [Fact] + public void Decode_16BitGrayscaleTransparency_IsBigEndian() + { + using MemoryStream stream = new(); + stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + WriteChunk(stream, "IHDR", CreateHeaderData(2, 1, 16, PngColorType.Grayscale, PngInterlaceMode.None)); + WriteChunk(stream, "tRNS", [0x01, 0x02]); + WriteChunk(stream, "IDAT", Compress([0, 0x01, 0x02, 0x02, 0x01])); + WriteChunk(stream, "IEND", []); + byte[] data = stream.ToArray(); + + Assert.Equal(new L16(0x0102), Image.Identify(data).Metadata.GetPngMetadata().TransparentColor?.ToPixel()); + + using Image image = Image.Load(data); + Assert.Equal(ushort.MinValue, image[0, 0].A); + Assert.Equal(ushort.MaxValue, image[1, 0].A); + } + + /// + /// 16-bit truecolor tRNS samples are big-endian, so (0x0102, 0x0304, 0x0506) is transparent and its byte-swapped + /// value is not. + /// + [Fact] + public void Decode_16BitRgbTransparency_IsBigEndian() + { + using MemoryStream stream = new(); + stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + WriteChunk(stream, "IHDR", CreateHeaderData(2, 1, 16, PngColorType.Rgb, PngInterlaceMode.None)); + WriteChunk(stream, "tRNS", [0x01, 0x02, 0x03, 0x04, 0x05, 0x06]); + WriteChunk(stream, "IDAT", Compress([0, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x02, 0x01, 0x04, 0x03, 0x06, 0x05])); + WriteChunk(stream, "IEND", []); + byte[] data = stream.ToArray(); + + Rgb48 expected = new(0x0102, 0x0304, 0x0506); + Assert.Equal(expected, Image.Identify(data).Metadata.GetPngMetadata().TransparentColor?.ToPixel()); + + using Image image = Image.Load(data); + Assert.Equal(ushort.MinValue, image[0, 0].A); + Assert.Equal(ushort.MaxValue, image[1, 0].A); + } + + /// + /// Creates an image filled with reproducible random colors. + /// + /// The image width. + /// The image height. + /// The random seed. + /// The . + private static Image CreateNoiseImage(int width, int height, int seed) + { + Random random = new(seed); + Image image = new(width, height); + for (int y = 0; y < height; y++) + { + for (int x = 0; x < width; x++) + { + image[x, y] = new Rgba32((byte)random.Next(256), (byte)random.Next(256), (byte)random.Next(256), (byte)random.Next(256)); + } + } + + return image; + } + + /// + /// Splits a PNG file into its chunks without validating them. + /// + /// The PNG file, including the signature. + /// The type and data of each chunk, in file order. + private static List<(string Type, byte[] Data)> ReadChunks(byte[] data) + { + List<(string Type, byte[] Data)> chunks = []; + int offset = 8; + while (offset + 12 <= data.Length) + { + int length = BinaryPrimitives.ReadInt32BigEndian(data.AsSpan(offset)); + chunks.Add((Encoding.ASCII.GetString(data, offset + 4, 4), data.AsSpan(offset + 8, length).ToArray())); + offset += length + 12; + } + + return chunks; + } + + /// + /// Writes a chunk with its length, type, data and a valid CRC. + /// + /// The stream to write to. + /// The four character chunk type. + /// The chunk data. + private static void WriteChunk(Stream stream, string type, ReadOnlySpan data) + { + Span buffer = stackalloc byte[4]; + BinaryPrimitives.WriteInt32BigEndian(buffer, data.Length); + stream.Write(buffer); + + // The CRC covers the chunk type and data but not the length. + byte[] typeBytes = Encoding.ASCII.GetBytes(type); + stream.Write(typeBytes); + stream.Write(data); + + Crc32 crc = new(); + crc.Append(typeBytes); + crc.Append(data); + BinaryPrimitives.WriteUInt32BigEndian(buffer, crc.GetCurrentHashAsUInt32()); + stream.Write(buffer); + } + + /// + /// Creates IHDR chunk data using compression method 0 and filter method 0. + /// + /// The image width. + /// The image height. + /// The bit depth. + /// The color type. + /// The interlace mode. + /// The chunk data. + private static byte[] CreateHeaderData(int width, int height, byte bitDepth, PngColorType colorType, PngInterlaceMode interlaceMode) + { + byte[] data = new byte[13]; + BinaryPrimitives.WriteInt32BigEndian(data, width); + BinaryPrimitives.WriteInt32BigEndian(data.AsSpan(4), height); + data[8] = bitDepth; + data[9] = (byte)colorType; + data[12] = (byte)interlaceMode; + return data; + } + + /// + /// Compresses filtered scanlines into a zlib stream. + /// + /// The filtered scanlines. + /// The zlib stream. + private static byte[] Compress(byte[] scanlines) + { + using MemoryStream output = new(); + using (ZLibStream zlib = new(output, CompressionLevel.Optimal, true)) + { + zlib.Write(scanlines); + } + + return output.ToArray(); + } + + /// + /// Gets the big-endian bytes of an APNG sequence number. + /// + /// The sequence number. + /// The encoded sequence number. + private static byte[] GetSequenceNumber(uint sequenceNumber) + { + byte[] data = new byte[4]; + BinaryPrimitives.WriteUInt32BigEndian(data, sequenceNumber); + return data; + } + private static string GetChunkTypeName(uint value) { byte[] data = new byte[4]; diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs index fdaa70e031..885b12a26c 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs @@ -354,4 +354,48 @@ public void ExcludeFilter_WithNone_DoesNotExcludeChunks() bytesSpan = bytesSpan[(4 + 4 + length + 4)..]; } } + + [Theory] + [InlineData(PngColorType.Grayscale)] + [InlineData(PngColorType.Rgb)] + public void TransparencyChunk_16Bit_IsWrittenBigEndian(PngColorType colorType) + { + // arrange + using Image input = new(2, 1); + PngMetadata metadata = input.Metadata.GetPngMetadata(); + metadata.ColorType = colorType; + metadata.BitDepth = PngBitDepth.Bit16; + metadata.TransparentColor = colorType == PngColorType.Grayscale + ? Color.FromPixel(new L16(0x0102)) + : Color.FromPixel(new Rgb48(0x0102, 0x0304, 0x0506)); + + byte[] expected = colorType == PngColorType.Grayscale + ? [0x01, 0x02] + : [0x01, 0x02, 0x03, 0x04, 0x05, 0x06]; + + using MemoryStream memStream = new(); + + // act + input.Save(memStream, new PngEncoder { ColorType = colorType, BitDepth = PngBitDepth.Bit16 }); + + // assert + byte[] data = memStream.ToArray(); + Span bytesSpan = data.AsSpan(8); // Skip header. + bool transparencyFound = false; + while (bytesSpan.Length > 0) + { + int length = BinaryPrimitives.ReadInt32BigEndian(bytesSpan[..4]); + PngChunkType type = (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(4, 4)); + if (type == PngChunkType.Transparency) + { + transparencyFound = true; + Assert.Equal(expected, bytesSpan.Slice(8, length).ToArray()); + } + + bytesSpan = bytesSpan[(4 + 4 + length + 4)..]; + } + + Assert.True(transparencyFound); + Assert.Equal(metadata.TransparentColor, Image.Identify(data).Metadata.GetPngMetadata().TransparentColor); + } }