Drop GIF frame rows outside the logical screen and stop on LZW codes past the code tables

src/ImageSharp/Formats/Gif/GifDecoderCore.cs

@@ -723,12 +723,14 @@ private void ReadFrameColors<TPixel>(
         using IMemoryOwner<byte> indicesRowOwner = this.memoryAllocator.Allocate<byte>(descriptor.Width);
         Span<byte> 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<TPixel>(
                     // 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,14 +761,31 @@ private void ReadFrameColors<TPixel>(
                         }
                     }
 
-                    writeY = Math.Min(interlaceY + descriptor.Top, image.Height);
+                    writeY = interlaceY + descriptorTop;
                     interlaceY += interlaceIncrement;
                 }
                 else
                 {
                     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

src/ImageSharp/Formats/Gif/LzwDecoder.cs

@@ -209,6 +209,16 @@ public void DecodePixelRow(Span<byte> 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;

tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs

@@ -24,6 +24,14 @@ public class GifDecoderTests
         TestImages.Gif.Giphy, TestImages.Gif.Kumin
     ];
 
+    /// <summary>
+    /// The global color table of the synthesized frame geometry test images.
+    /// </summary>
+    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<TPixel>(TestImageProvider<TPixel> provider)
@@ -442,6 +450,172 @@ public void Decode_MalformedApplicationExtension_ThrowsWithStrict()
         });
     }
 
+    [Fact]
+    public void Decode_LzwCodePastCodeTables_StopsDecodingFrame()
+    {
+        using Image<Rgba32> image = Image.Load<Rgba32>(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<byte> indices = Configuration.Default.MemoryAllocator.Allocate2D<byte>(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<Rgba32> image = Image.Load<Rgba32>(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]);
+            }
+        }
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    /// <returns>The encoded GIF.</returns>
+    private static byte[] CreateLzwCodePastCodeTablesGif()
+        => CreateGeometryTestGif(4, 4, 0, 0, 4, 4, false, [12, 0x02, 0x02, 0x10, 0x00]);
+
+    /// <summary>
+    /// Creates a GIF89a with the 4-entry <see cref="GeometryTestColorTable"/> as its global color table
+    /// and a single image descriptor without a local color table.
+    /// </summary>
+    /// <param name="screenWidth">The logical screen width.</param>
+    /// <param name="screenHeight">The logical screen height.</param>
+    /// <param name="left">The frame left position.</param>
+    /// <param name="top">The frame top position.</param>
+    /// <param name="width">The frame width.</param>
+    /// <param name="height">The frame height.</param>
+    /// <param name="interlaced">Whether the frame is interlaced.</param>
+    /// <param name="imageData">The image data, from the LZW minimum code size to the block terminator.</param>
+    /// <returns>The encoded GIF.</returns>
+    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();
+    }
+
+    /// <summary>
+    /// Gets the frame rows in the order an interlaced GIF stores them.
+    /// </summary>
+    /// <param name="height">The frame height.</param>
+    /// <returns>The frame row numbers in storage order.</returns>
+    private static int[] GetInterlacedRowOrder(int height)
+    {
+        List<int> 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];
+    }
+
+    /// <summary>
+    /// Writes a little-endian 16-bit value.
+    /// </summary>
+    /// <param name="stream">The stream to write to.</param>
+    /// <param name="value">The value to write.</param>
+    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