Menu

#204 Fix integer overflow and (unsigned int) truncation in gif2rgb.c

v1.0_(example)
open
nobody
5
2 days ago
2 days ago
No

Commit 4a731ea (Fix integer overflow and type safety issues, June 2026)
fixed dgif_lib.c, egif_lib.c, gifalloc.c, and quantize.c but left three
identical bugs in gif2rgb.c unfixed.

=== Bug 1: (unsigned int) truncation in LoadRGB() ===

Lines 63-68:
Size = ((long)Width) * Height * sizeof(GifByteType);
if ((RedBuffer = malloc((unsigned int)Size)) == NULL ||
(
GreenBuffer = malloc((unsigned int)Size)) == NULL ||
(*BlueBuffer = malloc((unsigned int)Size)) == NULL)

Size is unsigned long (64-bit on Linux x86-64) but all three malloc()
calls cast it to (unsigned int) (32-bit). For Width=65537, Height=65537:

  • Size (unsigned long) = 4,295,098,369 (correct)
  • (unsigned int)Size = 131,073 (truncated — 4 GB lost!)
  • malloc allocates 128 KB instead of 4 GB
  • The read loop then writes Width*Height bytes into it => heap overflow

Verified with AddressSanitizer:
ERROR: AddressSanitizer: heap-buffer-overflow
WRITE of size 4295098369 at addr 0 bytes after 131073-byte region

Also: the (long)Width cast is the same old pattern that commit 4a731ea
replaced with (size_t)Width in quantize.c — gif2rgb.c was missed.

=== Bug 2: int*int overflow in RGB2GIF() ===

Line ~219:
malloc(Width * Height * sizeof(GifByteType))

Width and Height are int. "Width * Height" evaluates as int*int before
the size_t sizeof() multiplication. For Width=46341, Height=46342:
46341 * 46342 = 2,147,534,622 > INT_MAX (2,147,483,647)
=> signed integer overflow (undefined behavior in C)

For Width=65537, Height=65537 the overflowed int wraps to 131,073,
so malloc allocates 128 KB while GifQuantizeBuffer processes 4 GB.

=== Bug 3: int Size in GIF2RGB() ===

int i, j, Size, Row, Col, Width, Height, ExtCode, Count;

Size holds GifFile->SWidth * sizeof(GifPixelType) — a byte count that
should be size_t for consistency with the rest of the post-4a731ea codebase.

=== Proposed fix ===

--- a/gif2rgb.c
+++ b/gif2rgb.c
@@ LoadRGB() @@
-unsigned long Size;
+size_t Size;

-Size = ((long)Width) * Height * sizeof(GifByteType);
+Size = (size_t)Width * Height * sizeof(GifByteType);

-if ((RedBuffer = (GifByteType )malloc((unsigned int)Size)) == NULL ||

  • (GreenBuffer = (GifByteType )malloc((unsigned int)Size)) == NULL ||
  • (BlueBuffer = (GifByteType )malloc((unsigned int)Size)) == NULL) {
    +if ((RedBuffer = (GifByteType )malloc(Size)) == NULL ||
  • (GreenBuffer = (GifByteType )malloc(Size)) == NULL ||
  • (BlueBuffer = (GifByteType )malloc(Size)) == NULL) {

@@ RGB2GIF() @@

  • malloc(Width * Height * sizeof(GifByteType))
  • malloc((size_t)Width * Height * sizeof(GifByteType))

@@ GIF2RGB() @@
-int i, j, Size, Row, Col, Width, Height, ExtCode, Count;
+int i, j, Row, Col, Width, Height, ExtCode, Count;
+size_t Size;

PoC source and ASan crash output attached.

1 Attachments

Discussion


Log in to post a comment.