Skip to content

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

Open
rootvector2 wants to merge 1 commit intodloebl: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 intodloebl: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
Copy link
Copy Markdown
Owner

dloebl commented Mar 15, 2026

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

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