From aeb0ad36ec0568423f0485f4004f97d638a2da03 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Thu, 12 Sep 2024 12:36:57 +0800 Subject: [PATCH] Fix out-of-bounds read access in GIF decoding Caught by lldb: * thread #1, stop reason = EXC_BAD_ACCESS (code=1, address=0x1080d6000) frame #0: 0x00000001000061a4 demo-sdl`_twin_gif_to_pixmap [inlined] gif_is_bgcolor(gif=0x00000001008b4200, color=) at image-gif.c:516:13 [opt] 513 514 static int gif_is_bgcolor(const twin_gif_t *gif, const uint8_t *color) 515 { -> 516 return !memcmp(&gif->palette->colors[gif->bgindex * 3], color, 3); 517 } 518 519 static void gif_rewind(twin_gif_t *gif) Target 0: (demo-sdl) stopped. (lldb) up frame #1: 0x000000010000619e demo-sdl`_twin_gif_to_pixmap at image-gif.c:584 [opt] 581 uint8_t r = *(color++); 582 uint8_t g = *(color++); 583 uint8_t b = *(color++); -> 584 if (!gif_is_bgcolor(gif, color)) 585 *(p.argb32++) = 0xFF000000U | (r << 16) | (g << 8) | b; 586 /* Construct background */ 587 else if (((row >> 3) + (col >> 3)) & 1) The removal of unnecessary pointer arithmetics also help compiler optimizations. --- mk/common.mk | 2 +- src/image-gif.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mk/common.mk b/mk/common.mk index fa99170..3c36914 100644 --- a/mk/common.mk +++ b/mk/common.mk @@ -117,7 +117,7 @@ RM := rm -rf MKDIR := mkdir -p __CFLAGS := -Wall -Wextra -pipe -__CFLAGS += -O2 -g -pipe +__CFLAGS += -Og -g -pipe __CFLAGS += $(CFLAGS) uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') diff --git a/src/image-gif.c b/src/image-gif.c index d9d82b8..99cc736 100644 --- a/src/image-gif.c +++ b/src/image-gif.c @@ -578,9 +578,7 @@ static twin_animation_t *_twin_animation_from_gif_file(const char *path) twin_pointer_t p = twin_pixmap_pointer(anim->frames[i], 0, 0); twin_coord_t row = 0, col = 0; for (int j = 0; j < gif->width * gif->height; j++) { - uint8_t r = *(color++); - uint8_t g = *(color++); - uint8_t b = *(color++); + uint8_t r = color[0], g = color[1], b = color[2]; if (!gif_is_bgcolor(gif, color)) *(p.argb32++) = 0xFF000000U | (r << 16) | (g << 8) | b; /* Construct background */ @@ -593,6 +591,8 @@ static twin_animation_t *_twin_animation_from_gif_file(const char *path) row++; col = 0; } + /* next palette */ + color += 3; } /* GIF delay in units of 1/100 second */ anim->frame_delays[i] = gif->gce.delay * 10;