Fixed CVE-2019-7635 and bug 4498 - Heap-Buffer Overflow in Blit1to4 pertaining to SDL_blit_1.c
authorSam Lantinga <slouken@libsdl.org>
Sat, 16 Mar 2019 18:34:22 -0700
changeset 63903bd33e8cb49
parent 638 e3e9d7430674
child 640 db8eb5e7247c
Fixed CVE-2019-7635 and bug 4498 - Heap-Buffer Overflow in Blit1to4 pertaining to SDL_blit_1.c

Petr Pisar

The root cause is that the POC BMP file declares 3 colors used and 4 bpp palette, but pixel at line 28 and column 1 (counted from 0) has color number 3. Then when the image loaded into a surface is passed to SDL_DisplayFormat(), in order to convert it to a video format, a used bliting function looks up a color number 3 in a 3-element long color bliting map. (The map obviously has the same number entries as the surface format has colors.)

Proper fix should refuse broken BMP images that have a pixel with a color index higher than declared number of "used" colors. Possibly more advanced fix could try to relocate the out-of-range color index into a vacant index (if such exists).
IMG_bmp.c
     1.1 --- a/IMG_bmp.c	Fri Jan 04 22:02:01 2019 -0800
     1.2 +++ b/IMG_bmp.c	Sat Mar 16 18:34:22 2019 -0700
     1.3 @@ -371,6 +371,14 @@
     1.4              ExpandBMP = biBitCount;
     1.5              biBitCount = 8;
     1.6              break;
     1.7 +        case 2:
     1.8 +        case 3:
     1.9 +        case 5:
    1.10 +        case 6:
    1.11 +        case 7:
    1.12 +            SDL_SetError("%d-bpp BMP images are not supported", biBitCount);
    1.13 +            was_error = SDL_TRUE;
    1.14 +            goto done;
    1.15          default:
    1.16              ExpandBMP = 0;
    1.17              break;
    1.18 @@ -505,48 +513,63 @@
    1.19          switch (ExpandBMP) {
    1.20              case 1:
    1.21              case 4: {
    1.22 -            Uint8 pixel = 0;
    1.23 -            int   shift = (8-ExpandBMP);
    1.24 -            for ( i=0; i<surface->w; ++i ) {
    1.25 -                if ( i%(8/ExpandBMP) == 0 ) {
    1.26 -                    if ( !SDL_RWread(src, &pixel, 1, 1) ) {
    1.27 -                        IMG_SetError("Error reading from BMP");
    1.28 +                Uint8 pixel = 0;
    1.29 +                int   shift = (8-ExpandBMP);
    1.30 +                for ( i=0; i<surface->w; ++i ) {
    1.31 +                    if ( i%(8/ExpandBMP) == 0 ) {
    1.32 +                        if ( !SDL_RWread(src, &pixel, 1, 1) ) {
    1.33 +                            IMG_SetError("Error reading from BMP");
    1.34 +                            was_error = SDL_TRUE;
    1.35 +                            goto done;
    1.36 +                        }
    1.37 +                    }
    1.38 +                    bits[i] = (pixel >> shift);
    1.39 +                    if (bits[i] >= biClrUsed) {
    1.40 +                        IMG_SetError("A BMP image contains a pixel with a color out of the palette");
    1.41                          was_error = SDL_TRUE;
    1.42                          goto done;
    1.43                      }
    1.44 +                    pixel <<= ExpandBMP;
    1.45                  }
    1.46 -                *(bits+i) = (pixel>>shift);
    1.47 -                pixel <<= ExpandBMP;
    1.48 -            } }
    1.49 +            }
    1.50              break;
    1.51  
    1.52              default:
    1.53 -            if ( SDL_RWread(src, bits, 1, surface->pitch) != surface->pitch ) {
    1.54 -                SDL_Error(SDL_EFREAD);
    1.55 -                was_error = SDL_TRUE;
    1.56 -                goto done;
    1.57 -            }
    1.58 +                if ( SDL_RWread(src, bits, 1, surface->pitch) != surface->pitch ) {
    1.59 +                    SDL_Error(SDL_EFREAD);
    1.60 +                    was_error = SDL_TRUE;
    1.61 +                    goto done;
    1.62 +                }
    1.63 +                if (biBitCount == 8 && palette && biClrUsed < (1 << biBitCount)) {
    1.64 +                    for (i = 0; i < surface->w; ++i) {
    1.65 +                        if (bits[i] >= biClrUsed) {
    1.66 +                            SDL_SetError("A BMP image contains a pixel with a color out of the palette");
    1.67 +                            was_error = SDL_TRUE;
    1.68 +                            goto done;
    1.69 +                        }
    1.70 +                    }
    1.71 +                }
    1.72  #if SDL_BYTEORDER == SDL_BIG_ENDIAN
    1.73 -            /* Byte-swap the pixels if needed. Note that the 24bpp
    1.74 -               case has already been taken care of above. */
    1.75 -            switch(biBitCount) {
    1.76 -                case 15:
    1.77 -                case 16: {
    1.78 -                    Uint16 *pix = (Uint16 *)bits;
    1.79 -                    for(i = 0; i < surface->w; i++)
    1.80 -                        pix[i] = SDL_Swap16(pix[i]);
    1.81 -                    break;
    1.82 +                /* Byte-swap the pixels if needed. Note that the 24bpp
    1.83 +                   case has already been taken care of above. */
    1.84 +                switch(biBitCount) {
    1.85 +                    case 15:
    1.86 +                    case 16: {
    1.87 +                        Uint16 *pix = (Uint16 *)bits;
    1.88 +                        for(i = 0; i < surface->w; i++)
    1.89 +                            pix[i] = SDL_Swap16(pix[i]);
    1.90 +                        break;
    1.91 +                    }
    1.92 +
    1.93 +                    case 32: {
    1.94 +                        Uint32 *pix = (Uint32 *)bits;
    1.95 +                        for(i = 0; i < surface->w; i++)
    1.96 +                            pix[i] = SDL_Swap32(pix[i]);
    1.97 +                        break;
    1.98 +                    }
    1.99                  }
   1.100 -
   1.101 -                case 32: {
   1.102 -                    Uint32 *pix = (Uint32 *)bits;
   1.103 -                    for(i = 0; i < surface->w; i++)
   1.104 -                        pix[i] = SDL_Swap32(pix[i]);
   1.105 -                    break;
   1.106 -                }
   1.107 -            }
   1.108  #endif
   1.109 -            break;
   1.110 +                break;
   1.111          }
   1.112          /* Skip padding bytes, ugh */
   1.113          if ( pad ) {