Skip to content

Fix 32-bit overflow in LZW size calculation (MAX_CODE_LEN * lzwPos)#108

Open
uwezkhan wants to merge 3 commits into
dloebl:mainfrom
uwezkhan:fix/lzw-size-multiplication-overflow
Open

Fix 32-bit overflow in LZW size calculation (MAX_CODE_LEN * lzwPos)#108
uwezkhan wants to merge 3 commits into
dloebl:mainfrom
uwezkhan:fix/lzw-size-multiplication-overflow

Conversation

@uwezkhan

@uwezkhan uwezkhan commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

This patch fixes a 32-bit arithmetic overflow in the LZW
byte list size calculations in cgif_raw.c.

The expression:

MAX_CODE_LEN * lzwPos

was evaluated using 32-bit arithmetic because
MAX_CODE_LEN is defined as an int and lzwPos is uint32_t.
Under C's usual arithmetic conversions, the multiplication
is performed as uint32_t before being widened to 64-bit
for the subsequent division.

If lzwPos is sufficiently large, the intermediate
32-bit multiplication can wrap around, resulting in an
incorrect (truncated) buffer size being passed to malloc().

The fix casts MAX_CODE_LEN to uint64_t:

(uint64_t)MAX_CODE_LEN * lzwPos

This ensures the multiplication is performed in 64-bit
arithmetic and prevents intermediate overflow.

No API changes.
No behavioral changes for valid inputs.
Negligible runtime impact.

@dloebl

dloebl commented Mar 15, 2026

Copy link
Copy Markdown
Owner

Hey @uwezkhan,
please read the contributing guidelines before opening any new pull requests or issues.

Missing test case (required): Bug fix PRs must include a test in tests/ that reproduces the issue on main (fails before the fix, passes after). No test was provided and no changes to tests/meson.build were made. See CONTRIBUTING.md:, section "Submitting Pull Requests", item 3.

AI tooling attribution: If any AI models or tooling were used to produce this PR, please disclose that per our contributing guidelines (CONTRIBUTING.md:, item 5).

At 19000x19000 the LZW code count landed just under UINT32_MAX/12, so the
32-bit product never wrapped and the test passed on unfixed main. Bump to
20000x20000 so lzwPos stays above the threshold and the overflow reproduces.
@uwezkhan

uwezkhan commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Added the reproducer as tests/lzw_size_overflow.c and registered it in tests/meson.build.

Proof of failure, same test run against both trees:

Before (unfixed main):

1/1 libcgif:lzw_size_overflow FAIL   killed by signal 11 SIGSEGV

After (this branch):

1/1 libcgif:lzw_size_overflow OK

The test encodes 20000x20000 pseudo-random pixels so the LZW code count stays above UINT32_MAX/12, which is where the 32-bit MAX_CODE_LEN * lzwPos product wraps and undersizes the byte-list malloc. One tradeoff worth flagging: it needs ~2.6 GB peak and runs in ~8s. That is inherent to this bug, since you have to push past ~358M codes to reach the wrap point.

On the tooling question: yeah, I lean on an LLM a bit for research and sanity-checking, nothing heavy.

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