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