Corrupted GIF header causes null pointer dereference by failed allocation due to integer overflow

[cwshugg]

Hi,

I discovered a bug in catimg's GIF parsing code that causes a segmentation fault from a null pointer dereference. (Commit hash 70e6f5ef627240589378e0e9e527a197faa0acde.) The bug can be revealed by passing catimg a GIF with unusually large width and height header fields. Here are links to two GIFs to help with recreating the bug:

1. GIF 1 is the original with which I found the bug. It's a fuzzed version of what was once a stock image.
2. GIF 2 is a 10-byte-long file only containing the GIF header. It's much smaller and will also cause the same segmentation fault.

Stacktrace

Throwing this into GDB shows the failure is occurring at src/stb_image.h:6492, where a memset() is attempting to write to g->history.

g->history is NULL, which causes the segmentation fault.

Root Cause

After doing some digging, I found that the stbi__gif struct's w and h fields (signed 32-bit integers) are multiplied together to determine the size of a heap allocation made within stbi__gif_load_next():

g->out = (stbi_uc *) stbi__malloc(4 * g->w * g->h);
g->background = (stbi_uc *) stbi__malloc(4 * g->w * g->h);
g->history = (stbi_uc *) stbi__malloc(g->w * g->h);

When catimg parses GIF 1, the resulting w and h fields read from its GIF header are 65535 and 33023. Since w and h are signed ints, the results of the above three multiplications are:

// g->w = 65535
// g->h = 33023
g->out = (stbi_uc *) stbi__malloc(4 * g->w * g->h); // (4 * g->w * g->h) = 66714628
g->background = (stbi_uc *) stbi__malloc(4 * g->w * g->h); // (4 * g->w * g->h) = 66714628
g->history = (stbi_uc *) stbi__malloc(g->w * g->h); // (g->w * g->h) = -2130804991

The problem lies in the calculation for g->history. Because of the nature of signed integers, the product of 65535 and 33023 results in an integer too large for the 32-bit maximum. Thus, it wraps around to be interpreted as a large negative value. (Interestingly, this doesn't occur for g->out and g->background, because multiplying the large-negative by 4 causes another wrap-around back into the positives.)

This negative integer is passed into stbi__malloc(), which interprets the parameter as a size_t (an unsigned integer). Examining this in GDB shows that the negative number is interpreted as a huge positive integer:

As a result, the call to STBI_MALLOC() (which is a macro for malloc()) fails, returning a NULL pointer.

[posva]

Seems to be in the external lib. Feel free to try to fix

[cwshugg]

Ah good point, I see the code's been patched in the stb lib!