#93 Buffer over-read in gifbuild due to missing bounds check on data after netscape extension block

[Hugh Davenport]

Found with afl-fuzz after bug #89 fixed, sorry I didn't get it before and file as one bug...

Due to not having a bounds check on the data after the NETSCAPE2.0 block, which required to have 3 bytes (params[1] and params[2], params[0] is ignored).

Input file.

 $ xxd -g 1 next-netscape-missing-bytes.gif 
0000000: 47 49 46 30 30 30 30 30 30 30 f2 30 30 30 30 30  GIF0000000.00000
0000010: 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30  0000000000000000
0000020: 30 30 30 30 30 21 ff 10 4e 45 54 53 43 41 50 45  00000!..NETSCAPE
0000030: 32 2e 30 30 30 30 30 30 02 30 30 04 30 30 30 30  2.000000.00.0000
0000040: 00 2c 30 30 30 30 00 00 30 30 30 03 8b 30 30 30  .,0000..000..000
0000050: 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30  0000000000000000
0000060: 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30  0000000000000000
0000070: 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30  0000000000000000
0000080: 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30  0000000000000000
0000090: 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30  0000000000000000
00000a0: 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30  0000000000000000
00000b0: 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30  0000000000000000
00000c0: 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30  0000000000000000
00000d0: 30 30 30 30 30 30 30 30 00 3b                    00000000.;

Crash.

$ ASAN_OPTIONS="abort_on_error=1 symbolize=1" ./util/.libs/lt-gifbuild -d next-netscape-missing-bytes.gif                                                           
#
# GIF information from next-netscape-missing-bytes.gif
screen width 12336
screen height 12336
screen colors 256
screen background 48
pixel aspect byte 48

screen map
        sort flag off
        rgb 048 048 048 is 0
        rgb 048 048 048 is 1
        rgb 048 048 048 is 2
        rgb 048 048 048 is 3
        rgb 048 048 048 is 4
        rgb 048 048 048 is 5
        rgb 048 048 048 is 6
        rgb 048 048 048 is 7
end

=================================================================
==17823== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60040000dfb2 at pc 0x404399 bp 0x7ffe316c3c20 sp 0x7ffe316c3c18
READ of size 1 at 0x60040000dfb2 thread T0
    #0 0x404398 in DumpExtensions.isra.0 /root/giflib-code/util/gifbuild.c:733
    #1 0x405395 in Gif2Icon.isra.1 /root/giflib-code/util/gifbuild.c:808
    #2 0x4020be in main /root/giflib-code/util/gifbuild.c:100
    #3 0x7f9c5f753ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
    #4 0x4025e1 in _start (/root/giflib-code/util/.libs/lt-gifbuild+0x4025e1)
0x60040000dfb2 is located 0 bytes to the right of 2-byte region [0x60040000dfb0,0x60040000dfb2)
allocated by thread T0 here:
    #0 0x7f9c5fd4141a (/usr/lib/x86_64-linux-gnu/libasan.so.0+0x1541a)
    #1 0x7f9c5fb1fd26 in GifAddExtensionBlock /root/giflib-code/lib/gifalloc.c:250
SUMMARY: AddressSanitizer: heap-buffer-overflow /root/giflib-code/util/gifbuild.c:733 DumpExtensions.isra.0
Shadow bytes around the buggy address:
  0x0c00ffff9ba0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c00ffff9bb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c00ffff9bc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c00ffff9bd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c00ffff9be0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa 01 fa
=>0x0c00ffff9bf0: fa fa 04 fa fa fa[02]fa fa fa 00 00 fa fa 03 fa
  0x0c00ffff9c00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c00ffff9c10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c00ffff9c20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c00ffff9c30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c00ffff9c40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:     fa
  Heap righ redzone:     fb
  Freed Heap region:     fd
  Stack left redzone:    f1
  Stack mid redzone:     f2
  Stack right redzone:   f3
  Stack partial redzone: f4
  Stack after return:    f5
  Stack use after scope: f8
  Global redzone:        f9
  Global init order:     f6
  Poisoned by user:      f7
  ASan internal:         fe
==17823== ABORTING
Aborted

Patch to fix, also attached.

diff --git a/util/gifbuild.c b/util/gifbuild.c
index 498d2e8..a5ed09d 100644
--- a/util/gifbuild.c
+++ b/util/gifbuild.c
@@ -728,6 +728,7 @@ static void DumpExtensions(GifFileType *GifFileOut,
        else if (!last
                 && ep->Function == APPLICATION_EXT_FUNC_CODE
                 && ep->ByteCount >= 11
+                && (ep+1)->ByteCount >= 3
                 && memcmp(ep->Bytes, "NETSCAPE2.0", 11) == 0) {
            unsigned char *params = (++ep)->Bytes;
            unsigned int loopcount = params[1] | (params[2] << 8);

[Hugh Davenport]

Sorry forgot to click private on this one...

[Eric S. Raymond]

Merged, thanks.