Skip to content

Fix integer overflow in RGB addframe byte-size calculation and add re…#106

Open
rootvector2 wants to merge 1 commit into
dloebl:mainfrom
rootvector2:rgb-addframe-size-overflow-checks
Open

Fix integer overflow in RGB addframe byte-size calculation and add re…#106
rootvector2 wants to merge 1 commit into
dloebl:mainfrom
rootvector2:rgb-addframe-size-overflow-checks

Conversation

@rootvector2

Copy link
Copy Markdown
Contributor

…gression test

@rootvector2

Copy link
Copy Markdown
Contributor Author

Hi @dloebl

I know in my previous PR I made wrong assumptions and did not include a proper test. This time I rechecked everything carefully.

I found an integer overflow in the RGB addframe size calculation. I reproduced it with large dimensions and added a minimal test file so you can verify the behavior directly.

I ran the full test suite and everything passes.

If anything still looks wrong please tell me and I will fix it properly.

@dloebl

dloebl commented Mar 15, 2026

Copy link
Copy Markdown
Owner

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

Passing test suite(required): The tests are still failing for your changes. Please fix them.

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

@rootvector2
rootvector2 force-pushed the rgb-addframe-size-overflow-checks branch 3 times, most recently from ffec0f6 to 8e7176a Compare March 15, 2026 21:30
The multiplication fmtChan * numPixel in quantize_and_dither() and
cgif_rgb_addframe() was computed in uint32_t arithmetic, which overflows
for large RGBA images (e.g. 32768x32769x4 > UINT32_MAX). This caused
undersized heap allocations followed by out-of-bounds writes.

Fix by promoting the multiplication to size_t with an overflow check.
Add regression test (rgb_overflow_trigger) that exercises the edge case.
@rootvector2
rootvector2 force-pushed the rgb-addframe-size-overflow-checks branch from 8e7176a to 238a742 Compare March 15, 2026 21:32
@rootvector2

Copy link
Copy Markdown
Contributor Author

@dloebl
test suite are paased

@rootvector2

Copy link
Copy Markdown
Contributor Author

ci went green after the last push, all seven jobs pass including the 32-bit build where the overflow actually fires.

on the tooling question: i lean on an ai tool now and then for research and to sanity-check my reasoning, but the patch here is mine. the root cause is real, fmtChan * numPixel was computed in uint32_t, so a large rgba frame wraps past UINT32_MAX and undersizes the malloc, then the memcpy runs off the end. i promoted the multiply to size_t and added an overflow guard. found it reading the addframe path while checking the earlier alloc fix. happy to reshape the check if you'd prefer a different style.

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