Skip to content

Commit 8e7176a

Browse files
committed
fix integer overflow in cgif_rgb_addframe byte-size calculation
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.
1 parent 436d5a4 commit 8e7176a

3 files changed

Lines changed: 72 additions & 4 deletions

File tree

src/cgif_rgb.c

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -542,12 +542,17 @@ static int quantize_and_dither(colHashTable* colhash, const uint8_t* pImageDataR
542542
if(root == NULL) {
543543
return -1;
544544
}
545-
float* pImageDataRGBfloat = malloc(fmtChan * numPixel * sizeof(float)); // TBD fmtChan + only when hasAlpha
545+
size_t numValues = (size_t)fmtChan * numPixel;
546+
if(numPixel != 0 && numValues / numPixel != fmtChan) {
547+
free_decision_tree(root);
548+
return -1;
549+
}
550+
float* pImageDataRGBfloat = malloc(numValues * sizeof(float)); // TBD fmtChan + only when hasAlpha
546551
if(pImageDataRGBfloat == NULL) {
547552
free_decision_tree(root);
548553
return -1;
549554
}
550-
for(uint32_t i = 0; i < fmtChan * numPixel; ++i){
555+
for(size_t i = 0; i < numValues; ++i){
551556
pImageDataRGBfloat[i] = pImageDataRGB[i];
552557
}
553558
uint8_t transIndex = colMax;
@@ -616,12 +621,17 @@ cgif_result cgif_rgb_addframe(CGIFrgb* pGIF, const CGIFrgb_FrameConfig* pConfig)
616621
pGIF->curResult = CGIF_ERROR;
617622
return CGIF_ERROR;
618623
}
619-
pNewBef = malloc(pConfig->fmtChan * MULU16(imageWidth, imageHeight));
624+
size_t totalBytes = (size_t)pConfig->fmtChan * numPixel;
625+
if(numPixel != 0 && totalBytes / numPixel != pConfig->fmtChan) {
626+
pGIF->curResult = CGIF_EALLOC;
627+
return pGIF->curResult;
628+
}
629+
pNewBef = malloc(totalBytes);
620630
if(pNewBef == NULL) {
621631
pGIF->curResult = CGIF_EALLOC;
622632
return pGIF->curResult;
623633
}
624-
memcpy(pNewBef, pConfig->pImageData, pConfig->fmtChan * MULU16(imageWidth, imageHeight));
634+
memcpy(pNewBef, pConfig->pImageData, totalBytes);
625635
fConfig.pLocalPalette = aPalette;
626636
fConfig.pImageData = malloc(pGIF->config.width * (uint32_t)pGIF->config.height);
627637
if(fConfig.pImageData == NULL) {

tests/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ tests_rgb = [
5757
{ 'name' : 'rgb_255colors', 'seed_should_fail' : false},
5858
{ 'name' : 'rgb_256colors', 'seed_should_fail' : false},
5959
{ 'name' : 'rgb_256digit', 'seed_should_fail' : false},
60+
{ 'name' : 'rgb_overflow_trigger', 'seed_should_fail' : false},
6061
{ 'name' : 'rgb_interlaced', 'seed_should_fail' : false},
6162
{ 'name' : 'rgb_single_color', 'seed_should_fail' : false},
6263
{ 'name' : 'rgb_no_dithering', 'seed_should_fail' : false},

tests/rgb_overflow_trigger.c

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
#include <stdlib.h>
2+
#include <stdint.h>
3+
#include <string.h>
4+
#include <stdio.h>
5+
6+
#include "cgif.h"
7+
8+
static int pWriteFn(void* pContext, const uint8_t* pData, const size_t numBytes) {
9+
(void)pContext;
10+
(void)pData;
11+
(void)numBytes;
12+
return 0;
13+
}
14+
15+
/* regression test: fmtChan * numPixel overflows uint32_t for large dimensions with RGBA.
16+
on 32-bit platforms size_t is 32 bits, so the overflow check fires and CGIF_EALLOC is returned.
17+
on 64-bit platforms the multiplication fits in size_t but processing a 4 GB image
18+
would time out on CI, so we skip. */
19+
int main(void) {
20+
if(sizeof(size_t) > 4) {
21+
return 0; // skip on 64-bit: no uint32_t overflow possible in size_t
22+
}
23+
24+
const uint16_t width = 32768;
25+
const uint16_t height = 32769; // width * height * 4 > UINT32_MAX
26+
27+
CGIFrgb_Config gConfig = {0};
28+
gConfig.pWriteFn = pWriteFn;
29+
gConfig.width = width;
30+
gConfig.height = height;
31+
32+
CGIFrgb* pGIF = cgif_rgb_newgif(&gConfig);
33+
if(pGIF == NULL) {
34+
return 2;
35+
}
36+
37+
size_t bufSize = (size_t)width * height * 4;
38+
uint8_t* buf = malloc(bufSize);
39+
if(buf == NULL) {
40+
cgif_rgb_close(pGIF);
41+
return 0; // skip: not enough memory
42+
}
43+
memset(buf, 0, bufSize);
44+
45+
CGIFrgb_FrameConfig fConfig = {0};
46+
fConfig.pImageData = buf;
47+
fConfig.fmtChan = CGIF_CHAN_FMT_RGBA;
48+
fConfig.delay = 0;
49+
50+
cgif_result r = cgif_rgb_addframe(pGIF, &fConfig);
51+
cgif_rgb_close(pGIF);
52+
free(buf);
53+
if(r == CGIF_EALLOC) {
54+
return 0; // overflow correctly detected
55+
}
56+
return 1;
57+
}

0 commit comments

Comments
 (0)