Skip to content

fix(gif): avoid int32 overflow in palette-split pixel-count math#5292

Merged
lgritz merged 3 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-gifpaletteoverflow
Jul 6, 2026
Merged

fix(gif): avoid int32 overflow in palette-split pixel-count math#5292
lgritz merged 3 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-gifpaletteoverflow

Conversation

@lgritz

@lgritz lgritz commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

GifSplitPalette() computed numPixels * (splitElt - firstElt) as a 32-bit int before dividing. For any image over roughly 16.9M pixels (e.g. 4117x4117), that intermediate multiplication overflows INT_MAX and is undefined behavior; in practice it produced a bogus negative/garbage split count that could drive later indexing into image out of its bounds.

Do the multiply in 64 bits and narrow back afterward -- the quotient is always <= numPixels, so the narrowing itself is safe.

Assisted-by: Claude Code / Sonnet 5

GifSplitPalette() computed `numPixels * (splitElt - firstElt)` as a
32-bit int before dividing. For any image over roughly 16.9M pixels
(e.g. 4117x4117), that intermediate multiplication overflows
INT_MAX and is undefined behavior; in practice it produced a bogus
negative/garbage split count that could drive later indexing into
`image` out of its bounds.

Do the multiply in 64 bits and narrow back afterward -- the quotient is
always <= numPixels, so the narrowing itself is safe.

Assisted-by: Claude Code / Sonnet 5

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@jessey-git

Copy link
Copy Markdown
Contributor

Given the max dimensions of 32768x32768x4, there are 4 other int overflows related to gif input, might as well do them in the same PR:
For gif.h

  • The GIF_MALLOC calculation inside GifBegin
  • The imageSize calculation inside GifMakePalette

For gifinput.cpp

  • The memcpy calculation inside GIFInput::read_native_scanline
  • The idx calculation inside GIFInput::read_subimage_data

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz

lgritz commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

So amended.

@jessey-git jessey-git left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but watch out for clang-format.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz lgritz merged commit 134977d into AcademySoftwareFoundation:main Jul 6, 2026
28 checks passed
@lgritz lgritz deleted the lg-gifpaletteoverflow branch July 6, 2026 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants