clang and gif

I wrote yesterday about using clangs -faddress=sanitize feature.
Boy - is that brilliant. I have fixed maybe 20 buffer overflow/underflow
bugs in my code. Most of the bugs are benign - but a few
highlighted real errors (such as tagging SQL files could leave garbage
at the end of a definition).

This one is strange. It highlights a bug in GIF code. As far as I can
tell - most GIF implementations are based on the same example code from
the X11 consortium. The bug appears to be benign - but it is naughty.

static int
GetCode(file_info_t *fip, int code_size, int flag, int *errp)
{
    static unsigned char    buf[280];
    static int        curbit, lastbit, done, last_byte;
    int            i, j, ret;
    unsigned char        count;

    *errp = FALSE;

    if (flag) {
        curbit = 0;
        lastbit = 0;
        done = FALSE;
        return 0;
    }

    if ( (curbit+code_size) >= lastbit) {
        if (done) {
            /* if (curbit >= lastbit) then ran off the end of bits */
            return -1;
        }

        /* BUG IS HERE - Original code. last_byte will be zero */
	/* so we read two bytes before the static buffer, above. */
        // buf[0] = buf[last_byte-2];
        // buf[1] = buf[last_byte-1];

        buf[0] = last_byte >= 2 ? buf[last_byte-2] : 0;
        buf[1] = last_byte >= 1 ? buf[last_byte-1] : 0;

Here is the error - I love the user of color in clang:

=================================================================
==16171==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00000176c83e at pc 0x8ec684 bp 0x7fffffff2a70 sp 0x7fffffff2a68
READ of size 1 at 0x00000176c83e thread T0
....stack dump...
0x00000176c83e is located 2 bytes to the left of global variable 'GetCode.buf' from 'gif.c' (0x176c840) of size 280                                             0x00000176c83e is located 54 bytes to the right of global variable 'LWZReadByte.sp' from 'gif.c' (0x176c800) of size 8
Shadow bytes around the buggy address:
  0x0000802e58b0: 00 00 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
  0x0000802e58c0: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
  0x0000802e58d0: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
  0x0000802e58e0: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
  0x0000802e58f0: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
=>0x0000802e5900: 00 f9 f9 f9 f9 f9 f9[f9]00 00 00 00 00 00 00 00
  0x0000802e5910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000802e5920: 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9 f9
  0x0000802e5930: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0000802e5940: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0000802e5950: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
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 right 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
==16171==ABORTING

This one testing feature (of clang) is really brilliant. Kudos to the
developers for helping improve software quality. I do hope
everyone else is now using it !

Post created by CRiSP v11.0.31a-b6729