Repository navigation
fix: prevent heap buffer overflow from integer overflow in cgif_rgb - #105
rootvector2 wants to merge 2 commits into
Conversation
The multiplication `fmtChan * numPixel` in quantize_and_dither() and cgif_rgb_addframe() is evaluated as uint32_t arithmetic, which silently wraps past UINT32_MAX for large user-controlled dimensions (e.g. width=32768, height=32769, fmtChan=4). This causes undersized heap allocations followed by out-of-bounds writes during Floyd-Steinberg dithering. - Add SIZE_MAX overflow guard before allocation in cgif_rgb_addframe() - Cast fmtChan * numPixel products to size_t to prevent truncation - Add #include <stdint.h> for SIZE_MAX
|
Hey @rootvector2, The fix addresses a real concern, but the submission doesn't meet our contribution requirements as described in CONTRIBUTING.md: Missing test case (required): Bug fix PRs must include a test in Unrelated change: The diff removes the trailing newline at the end of 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). |
|
Thanks for the review. I’ve addressed the feedback: |
| /* Provide a small dummy buffer. The library must reject the oversized | ||
| dimensions before attempting to read the full width*height*fmtChan | ||
| bytes from this pointer. */ | ||
| uint8_t dummy[4] = {0}; |
There was a problem hiding this comment.
That doesn't look right at all. CGIF shouldn't just "reject" images with larger dimensions.
|
The test is failing on this branch and it doesn't look right at all. Please read the CONTRIBUTING.md first. If I have to ask you again, I'll simply close this PR. |
|
Ok |
The multiplication
fmtChan * numPixelin quantize_and_dither() and cgif_rgb_addframe() is evaluated as uint32_t arithmetic, which silently wraps past UINT32_MAX for large user-controlled dimensions (e.g. width=32768, height=32769, fmtChan=4). This causes undersized heap allocations followed by out-of-bounds writes during Floyd-Steinberg dithering.