From 777d74f806f7e9241336efd1c76865c07e2f0436 Mon Sep 17 00:00:00 2001 From: everestsummer Date: Sat, 20 Aug 2022 13:21:07 +0800 Subject: [PATCH 1/6] Fix: Security: Uninitialized variables may cause SIGSEGV --- gifdec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gifdec.c b/gifdec.c index 83c2d0f..61448ec 100644 --- a/gifdec.c +++ b/gifdec.c @@ -333,12 +333,12 @@ static int read_image_data(gd_GIF *gif, int interlace) { uint8_t sub_len, shift, byte; - int init_key_size, key_size, table_is_full; - int frm_off, frm_size, str_len, i, p, x, y; + int init_key_size, key_size, table_is_full = 0; + int frm_off, frm_size, str_len = 0, i, p, x, y; uint16_t key, clear, stop; int ret; Table *table; - Entry entry; + Entry entry = { 0 }; off_t start, end; read(gif->fd, &byte, 1); From f29dc41102ea62f4d6a1c9e9438e8addf81fb1ab Mon Sep 17 00:00:00 2001 From: everestsummer Date: Sat, 20 Aug 2022 13:25:23 +0800 Subject: [PATCH 2/6] Fix: Security: 'key' maybe a value bigger than table->nentries, causing out of bounds read access. --- gifdec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gifdec.c b/gifdec.c index 61448ec..8f0e2db 100644 --- a/gifdec.c +++ b/gifdec.c @@ -379,6 +379,7 @@ read_image_data(gd_GIF *gif, int interlace) key = get_key(gif, key_size, &sub_len, &shift, &byte); if (key == clear) continue; if (key == stop || key == 0x1000) break; + if (key >= table->nentries) break; if (ret == 1) key_size++; entry = table->entries[key]; str_len = entry.length; From 970558212348b60359f967a5d999078b6f9efe04 Mon Sep 17 00:00:00 2001 From: everestsummer Date: Sat, 20 Aug 2022 13:36:15 +0800 Subject: [PATCH 3/6] Fix: Security: entry.prefix may be a bigger value than table->nentries, causing oob read. --- gifdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gifdec.c b/gifdec.c index 8f0e2db..c6718e1 100644 --- a/gifdec.c +++ b/gifdec.c @@ -390,7 +390,7 @@ read_image_data(gd_GIF *gif, int interlace) if (interlace) y = interlaced_line_index((int) gif->fh, y); gif->frame[(gif->fy + y) * gif->width + gif->fx + x] = entry.suffix; - if (entry.prefix == 0xFFF) + if (entry.prefix == 0xFFF || entry.prefix >= table->nentries) break; else entry = table->entries[entry.prefix]; From bdfad6b169f758a0c74cf7ddde7f591cfd8ef248 Mon Sep 17 00:00:00 2001 From: everestsummer Date: Sat, 20 Aug 2022 15:43:10 +0800 Subject: [PATCH 4/6] Fix: Security: Infinite loop in discard_sub_blocks --- gifdec.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gifdec.c b/gifdec.c index c6718e1..de1e303 100644 --- a/gifdec.c +++ b/gifdec.c @@ -121,11 +121,17 @@ gd_open_gif(const char *fname) static void discard_sub_blocks(gd_GIF *gif) { + uint8_t first_try = 1; + uint8_t seek_pos; uint8_t size; do { read(gif->fd, &size, 1); + if (!first_try && size == seek_pos) //To prevent infinite loop + break; lseek(gif->fd, size, SEEK_CUR); + seek_pos = size; + first_try = 0; } while (size); } From 271d1d22ce3d86ecddbd0f9205f0b70fa9fd7529 Mon Sep 17 00:00:00 2001 From: everestsummer Date: Sat, 20 Aug 2022 15:43:58 +0800 Subject: [PATCH 5/6] Fix: Security: Infinite loop in read_ext --- gifdec.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gifdec.c b/gifdec.c index de1e303..dd2d5b3 100644 --- a/gifdec.c +++ b/gifdec.c @@ -224,7 +224,8 @@ read_ext(gd_GIF *gif) { uint8_t label; - read(gif->fd, &label, 1); + if(read(gif->fd, &label, 1) < 1) + return; switch (label) { case 0x01: read_plain_text_ext(gif); @@ -502,7 +503,8 @@ gd_get_frame(gd_GIF *gif) if (sep == '!') read_ext(gif); else return -1; - read(gif->fd, &sep, 1); + if(read(gif->fd, &sep, 1) < 1) + return -1; } if (read_image(gif) == -1) return -1; From b17f41093397ef0b698dd718ab109625716414fe Mon Sep 17 00:00:00 2001 From: everestsummer Date: Sat, 20 Aug 2022 16:44:41 +0800 Subject: [PATCH 6/6] Fix: Security: Prevent i from being overflowed to negative value (and SIGSEGV) --- gifdec.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/gifdec.c b/gifdec.c index dd2d5b3..75bfc7d 100644 --- a/gifdec.c +++ b/gifdec.c @@ -44,7 +44,7 @@ gd_open_gif(const char *fname) uint8_t sigver[3]; uint16_t width, height, depth; uint8_t fdsz, bgidx, aspect; - int i; + size_t i; uint8_t *bgcolor; int gct_sz; gd_GIF *gif; @@ -121,13 +121,13 @@ gd_open_gif(const char *fname) static void discard_sub_blocks(gd_GIF *gif) { - uint8_t first_try = 1; + uint8_t first_try = 1; uint8_t seek_pos; uint8_t size; do { read(gif->fd, &size, 1); - if (!first_try && size == seek_pos) //To prevent infinite loop + if (!first_try && size == seek_pos) //To prevent infinite loop break; lseek(gif->fd, size, SEEK_CUR); seek_pos = size; @@ -225,7 +225,8 @@ read_ext(gd_GIF *gif) uint8_t label; if(read(gif->fd, &label, 1) < 1) - return; + return; + switch (label) { case 0x01: read_plain_text_ext(gif); @@ -504,7 +505,7 @@ gd_get_frame(gd_GIF *gif) read_ext(gif); else return -1; if(read(gif->fd, &sep, 1) < 1) - return -1; + return -1; } if (read_image(gif) == -1) return -1;