Menu

#37 Comprehensive integer overflow prevention and security hardening

Unstable_(example)
open
nobody
None
5
3 days ago
3 days ago
No

Summary

This patch fixes 4 integer overflow vulnerabilities in giflib's core library and adds security testing infrastructure (sanitizer build targets + libFuzzer harness).


Bug 1: Signed integer overflow in GifApplyTranslation (gifalloc.c:214)

Height * Width is computed as int (signed 32-bit). For images where the product exceeds INT_MAX, this is undefined behavior per C99 §6.5/5.

UBSan trace on unpatched giflib 6.1.2:

gifalloc.c:214:15: runtime error: signed integer overflow:
46341 * 46341 cannot be represented in type 'int'

The loop bound RasterSize gets an unpredictable value due to UB, corrupting pixel translation. Fixed by using size_t with a new GifSafeMult() overflow-checked multiplication helper.


Bug 2: Heap buffer overflow in GifMakeSavedImage (gifalloc.c:372-384)

Two independent overflow sites in the same function:

  1. reallocarray(NULL, (Height * Width), sizeof(GifPixelType)) — the Height * Width multiplication happens in int arithmetic before being passed to reallocarray, defeating its overflow protection entirely. The overflowed int is implicitly converted to size_t, producing a wrong allocation size.

  2. memcpy(dst, src, sizeof(GifPixelType) * Height * Width) — computes the copy size independently, also in int arithmetic. The memcpy size does not match the allocation size — on overflow, memcpy writes past the allocated buffer → heap buffer overflow.

Fixed by computing the size once with GifSafeMult() and using it for both allocation and copy. On overflow, the function returns NULL instead of allocating a wrong-sized buffer.


Bug 3: PixelCount overflow on 32-bit systems (dgif_lib.c:418, egif_lib.c:451)

(long)Width * (long)Height overflows signed long on 32-bit platforms where long is 32 bits. For example: 65535 × 65535 = 4,294,836,225 > LONG_MAX (2,147,483,647) on 32-bit.

PixelCount gets a wrong value, causing the decoder/encoder to process the wrong number of pixels. Fixed by casting to size_t and changing the PixelCount field type from unsigned long to size_t.

Note: PixelCount is in GifFilePrivateType (private header gif_lib_private.h), not the public API. Users access it through void *Private in GifFileType. This is not an ABI break.


Bug 4: Unsafe allocation in quantize.c

malloc(sizeof(QuantizedColorType *) * NumEntries) replaced with reallocarray(NULL, NumEntries, sizeof(QuantizedColorType *)) for consistent overflow-safe allocation.


Security Infrastructure Added

  • GifSafeMult() — overflow-checked size_t multiplication helper in gif_lib_private.h
  • Sanitizer Makefile targets: make check-asan, make check-ubsan, make check-sanitizers
  • libFuzzer harness: fuzz/gif_fuzz_dgif.c for the DGifSlurp decode path

Testing

  • All 51 existing tests pass (normal build)
  • All 51 tests pass under UBSan — zero undefined behavior errors
  • All 51 tests pass under ASan — zero memory errors
  • Fuzz harness runs clean at ~5000 exec/s with 165 edge coverage

Files Changed

Makefile             | 18 changes
dgif_lib.c           |  4 changes
egif_lib.c           |  2 changes
gif_lib_private.h    | 15 changes
gifalloc.c           | 41 changes
quantize.c           |  6 changes
fuzz/gif_fuzz_dgif.c | 63 lines (new file)
.gitignore           |  2 lines (new file)

Patch attached. Apply with:

git am giflib-security-hardening.patch
1 Attachments

Discussion


Log in to post a comment.

MongoDB Logo MongoDB