Skip to content

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

Open
jafin wants to merge 1 commit into
SixLabors:mainfrom
jafin:jf/gif-lzw-and-frame-geometry
Open

jafin wants to merge 1 commit into
SixLabors:mainfrom
jafin:jf/gif-lzw-and-frame-geometry

Conversation

@jafin

@jafin jafin commented Oct 17, 2026 •

Copy link
Copy Markdown

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Some corrupt or non-conforming GIFs made the decoder throw IndexOutOfRangeException or ArgumentOutOfRangeException instead of decoding the image or throwing InvalidImageContentException.

LZW codes past the code tables

With an LZW minimum code size of 12, the clear code is 4096. The first code after a clear code (or at the start of the data) can then be the next available code, 4098. That code is past the end of the 4096-entry prefix/suffix tables, so LzwDecoder.DecodePixelRow and LzwDecoder.SkipIndices threw IndexOutOfRangeException. Image.Load and Image.Identify both failed.

Both methods now stop decoding at such a code. The check is in the branch that handles the first code after a clear code, so it adds no per-pixel cost.

Frames that extend past the logical screen

  • For an interlaced frame whose lines fall below the logical screen, GifDecoderCore.ReadFrameColors clamped the target row to image.Height. That row is out of range, so the decoder threw ArgumentOutOfRangeException from DangerousGetRowSpan. Examples are a 20x20 interlaced frame on a 10x10 screen, or a 10x10 interlaced frame at top 5. The row loop was also limited by the screen height. For an interlaced frame that limit cuts off lines of later passes that are still on the screen.
  • An interlaced frame shorter than 5 lines has interlace passes with no lines. The decoder entered such a pass and wrote the frame's lines to the wrong rows, for example a 2-line frame wrote its second line to row 4.
  • A frame whose left edge is at or past the right edge of the screen threw ArgumentOutOfRangeException when the row was sliced.

Now the row loop covers every frame line. Lines below the screen are dropped. For an interlaced frame they are decoded and discarded only while a later pass can still reach the screen, and decoding stops after that. Empty interlace passes are skipped. A frame that starts at or past the right or bottom edge of the screen is not decoded; the existing sub-block skipping consumes its data. Sequential frames are clipped as before.

Valid GIFs, including the existing deferred clear code and bad descriptor dimension test images, decode as before. Their reference-output comparisons are unchanged.

Testing

New tests in GifDecoderTests use GIFs built in memory, so no new image files are added:

  • Decode_LzwCodePastCodeTables_StopsDecodingFrame and Identify_LzwCodePastCodeTables_ReturnsImageInfo cover the full decode path and the index skipping path.
  • Decode_FrameExceedingLogicalScreen_WritesOnlyRowsInsideCanvas encodes interlaced and sequential frames with the internal LzwEncoder. The frames have different sizes, top offsets (including at the bottom edge and with a single visible line) and left offsets past the right edge. The test checks that every on-screen pixel has the right color and that pixels outside the frame are untouched.

Before the change, 11 of the 17 new test cases failed, and each defect above had at least one failing case. The other cases pin behavior that already worked, such as sequential frames below the screen. All GIF tests pass in Release on net10.0 (156 in Formats.Gif, plus 10 GIF-related tests in other areas).

Fixes #3236

…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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant