From 89ca470b2feca29b0a4c8819748e3a1eb2fa6d4c Mon Sep 17 00:00:00 2001 From: Jason Finch Date: Sun, 18 Oct 2026 09:07:46 +1000 Subject: [PATCH] Drop GIF frame rows outside the logical screen and stop on LZW codes past the code tables An interlaced frame whose lines fall below the logical screen clamped the target row to the screen height and threw from the row lookup, frames shorter than five lines wrote their lines to the wrong rows, and a frame starting right of the screen threw while slicing the row. Lines below the screen are now decoded and discarded only while a later interlace pass can still reach the screen, and frames starting outside the screen are skipped. With a minimum code size of 12 the first code after a clear code could index past the 4096-entry LZW tables. Decoding and skipping now stop at such a code. --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 30 ++- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 20 ++ .../Formats/Gif/GifDecoderTests.cs | 174 ++++++++++++++++++ 3 files changed, 220 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 0a06f4915b..912a90b60d 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -723,12 +723,14 @@ private void ReadFrameColors( using IMemoryOwner indicesRowOwner = this.memoryAllocator.Allocate(descriptor.Width); Span indicesRow = indicesRowOwner.Memory.Span; + // A frame that starts at or beyond the right or bottom edge of the logical screen has nothing to draw. + // Its image data is then left to the caller, which skips the remaining data sub-blocks. int minCodeSize = stream.ReadByte(); - if (LzwDecoder.IsValidMinCodeSize(minCodeSize)) + if (descriptorTop < imageHeight && descriptorLeft < imageWidth && LzwDecoder.IsValidMinCodeSize(minCodeSize)) { using LzwDecoder lzwDecoder = new(this.configuration.MemoryAllocator, stream, minCodeSize); - for (int y = descriptorTop; y < descriptorBottom && y < imageHeight; y++) + for (int y = descriptorTop; y < descriptorBottom; y++) { // Check if this image is interlaced. int writeY; // the target y offset to write to @@ -737,7 +739,10 @@ private void ReadFrameColors( // If so then we read lines at predetermined offsets. // When an entire image height worth of offset lines has been read we consider this a pass. // With each pass the number of offset lines changes and the starting line changes. - if (interlaceY >= descriptor.Height) + // A frame shorter than 5 lines has passes without any lines, so keep advancing until a + // pass has a line left. The four passes visit every frame line exactly once and the loop + // runs once per frame line, so a pass with a line left always exists. + while (interlaceY >= descriptor.Height) { interlacePass++; switch (interlacePass) @@ -756,7 +761,7 @@ private void ReadFrameColors( } } - writeY = Math.Min(interlaceY + descriptor.Top, image.Height); + writeY = interlaceY + descriptorTop; interlaceY += interlaceIncrement; } else @@ -764,6 +769,23 @@ private void ReadFrameColors( writeY = y; } + // For a properly encoded gif the frame never extends below the logical screen. When it does + // the lines below the screen are dropped. Lines only move down within a pass, so once a line + // falls below the screen the rest of the pass is hidden too. A sequential frame is a single + // pass, so decoding can stop there. Before the last interlace pass, later passes still start + // at frame line 1, 2 or 4 and can reach the screen again unless it shows only frame line 0. + // The hidden lines in between must still be decoded and discarded to reach those lines. + if (writeY >= imageHeight) + { + if (!descriptor.InterlaceFlag || interlacePass == 3 || imageHeight - descriptorTop < 2) + { + break; + } + + lzwDecoder.SkipIndices(descriptor.Width); + continue; + } + lzwDecoder.DecodePixelRow(indicesRow); // #403 The left + width value can be larger than the image width diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index b33d1f3d48..6724e33e86 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -209,6 +209,16 @@ public void DecodePixelRow(Span indices) if (oldCode == NullCode) { + // The only non-literal code that can reach this point is the next available code. With a + // minimum code size of 12 the clear code is 4096, so that code lies past the end of the + // 4096-entry code tables. It has no table entry to output, so the data is corrupt and + // decoding stops. This branch only runs for the first code of the data or after a clear + // code, so the check adds no per-pixel cost. + if (code >= MaxStackSize) + { + break; + } + pixelStack[top++] = suffix[code]; oldCode = code; first = code; @@ -350,6 +360,16 @@ public void SkipIndices(int length) if (oldCode == NullCode) { + // The only non-literal code that can reach this point is the next available code. With a + // minimum code size of 12 the clear code is 4096, so that code lies past the end of the + // 4096-entry code tables. It has no table entry to output, so the data is corrupt and + // decoding stops. This branch only runs for the first code of the data or after a clear + // code, so the check adds no per-pixel cost. + if (code >= MaxStackSize) + { + break; + } + pixelStack[top++] = suffix[code]; oldCode = code; first = code; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 8a8b4866a2..a26fcd2b1d 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -24,6 +24,14 @@ public class GifDecoderTests TestImages.Gif.Giphy, TestImages.Gif.Kumin ]; + /// + /// The global color table of the synthesized frame geometry test images. + /// + private static readonly Rgba32[] GeometryTestColorTable = + [ + new(255, 0, 0), new(0, 255, 0), new(0, 0, 255), new(255, 255, 255) + ]; + [Theory] [WithFileCollection(nameof(MultiFrameTestFiles), PixelTypes.Rgba32)] public void Decode_VerifyAllFrames(TestImageProvider provider) @@ -442,6 +450,172 @@ public void Decode_MalformedApplicationExtension_ThrowsWithStrict() }); } + [Fact] + public void Decode_LzwCodePastCodeTables_StopsDecodingFrame() + { + using Image image = Image.Load(CreateLzwCodePastCodeTablesGif()); + + // No index is decoded so every pixel keeps the cleared index 0. + Assert.Equal(4, image.Width); + Assert.Equal(4, image.Height); + for (int y = 0; y < image.Height; y++) + { + for (int x = 0; x < image.Width; x++) + { + Assert.Equal(GeometryTestColorTable[0], image[x, y]); + } + } + } + + [Fact] + public void Identify_LzwCodePastCodeTables_ReturnsImageInfo() + { + // Identify skips the frame indices so this exercises the index skipping path of the LZW decoder. + ImageInfo info = Image.Identify(CreateLzwCodePastCodeTablesGif()); + + Assert.Equal(4, info.Width); + Assert.Equal(4, info.Height); + } + + [Theory] + [InlineData(20, 20, 0, 0, true)] + [InlineData(20, 20, 0, 0, false)] + [InlineData(10, 10, 0, 5, true)] + [InlineData(10, 10, 0, 5, false)] + [InlineData(10, 10, 5, 3, true)] + [InlineData(10, 10, 0, 10, true)] + [InlineData(10, 10, 0, 10, false)] + [InlineData(10, 20, 0, 8, true)] + [InlineData(10, 20, 0, 9, true)] + [InlineData(10, 1, 0, 0, true)] + [InlineData(10, 2, 0, 0, true)] + [InlineData(10, 3, 0, 7, true)] + [InlineData(10, 4, 0, 0, true)] + [InlineData(4, 4, 12, 0, true)] + [InlineData(4, 4, 12, 0, false)] + public void Decode_FrameExceedingLogicalScreen_WritesOnlyRowsInsideCanvas(int width, int height, int left, int top, bool interlaced) + { + const int screenSize = 10; + + // Each frame row is filled with the color index of its frame row number modulo 4, + // stored in the order an interlaced or sequential frame stores its rows. + int[] rowOrder = interlaced ? GetInterlacedRowOrder(height) : Enumerable.Range(0, height).ToArray(); + using Buffer2D indices = Configuration.Default.MemoryAllocator.Allocate2D(width, height); + for (int i = 0; i < rowOrder.Length; i++) + { + indices.DangerousGetRowSpan(i).Fill((byte)(rowOrder[i] % 4)); + } + + using MemoryStream imageData = new(); + using (LzwEncoder encoder = new(Configuration.Default.MemoryAllocator, 2)) + { + encoder.Encode(indices, imageData); + } + + byte[] data = CreateGeometryTestGif(screenSize, screenSize, left, top, width, height, interlaced, imageData.ToArray()); + + using Image image = Image.Load(data); + + Assert.Equal(screenSize, image.Width); + Assert.Equal(screenSize, image.Height); + for (int y = 0; y < image.Height; y++) + { + for (int x = 0; x < image.Width; x++) + { + // Pixels outside the frame are never written and stay transparent. + bool inFrame = x >= left && x < left + width && y >= top && y < top + height; + Rgba32 expected = inFrame ? GeometryTestColorTable[(y - top) % 4] : default; + Assert.Equal(expected, image[x, y]); + } + } + } + + /// + /// Creates a 4x4 GIF whose frame uses a minimum code size of 12 and holds the single 13-bit code 4098. + /// The clear code is 4096, so the code lies past the 4096-entry LZW code tables. + /// + /// The encoded GIF. + private static byte[] CreateLzwCodePastCodeTablesGif() + => CreateGeometryTestGif(4, 4, 0, 0, 4, 4, false, [12, 0x02, 0x02, 0x10, 0x00]); + + /// + /// Creates a GIF89a with the 4-entry as its global color table + /// and a single image descriptor without a local color table. + /// + /// The logical screen width. + /// The logical screen height. + /// The frame left position. + /// The frame top position. + /// The frame width. + /// The frame height. + /// Whether the frame is interlaced. + /// The image data, from the LZW minimum code size to the block terminator. + /// The encoded GIF. + private static byte[] CreateGeometryTestGif( + int screenWidth, + int screenHeight, + int left, + int top, + int width, + int height, + bool interlaced, + byte[] imageData) + { + using MemoryStream stream = new(); + stream.Write("GIF89a"u8); + + // Logical screen descriptor flagging a 4-entry global color table. + WriteUInt16(stream, screenWidth); + WriteUInt16(stream, screenHeight); + stream.Write([0x81, 0x00, 0x00]); + foreach (Rgba32 color in GeometryTestColorTable) + { + stream.Write([color.R, color.G, color.B]); + } + + // Image descriptor without a local color table. + stream.WriteByte(GifConstants.ImageLabel); + WriteUInt16(stream, left); + WriteUInt16(stream, top); + WriteUInt16(stream, width); + WriteUInt16(stream, height); + stream.WriteByte((byte)(interlaced ? 0x40 : 0x00)); + + stream.Write(imageData); + stream.WriteByte(GifConstants.EndIntroducer); + return stream.ToArray(); + } + + /// + /// Gets the frame rows in the order an interlaced GIF stores them. + /// + /// The frame height. + /// The frame row numbers in storage order. + private static int[] GetInterlacedRowOrder(int height) + { + List rows = []; + foreach ((int start, int step) in new[] { (0, 8), (4, 8), (2, 4), (1, 2) }) + { + for (int y = start; y < height; y += step) + { + rows.Add(y); + } + } + + return [.. rows]; + } + + /// + /// Writes a little-endian 16-bit value. + /// + /// The stream to write to. + /// The value to write. + private static void WriteUInt16(Stream stream, int value) + { + stream.WriteByte((byte)value); + stream.WriteByte((byte)(value >> 8)); + } + private static byte[] CreateGifWithMalformedApplicationExtension() { // The application extension declares a 5-byte application block (0x05), but GIF application