Fixed bug 4536 - Heap-Buffer Overflow in SDL_GetRGB pertaining to SDL_pixels.c
authorSam Lantinga <slouken@libsdl.org>
Tue, 03 Sep 2019 11:55:20 -0700
changeset 130526203d73874ab
parent 13050 0ff6de20d4f9
child 13056 ed80f97ebe81
Fixed bug 4536 - Heap-Buffer Overflow in SDL_GetRGB pertaining to SDL_pixels.c

Ozkan Sezer

As for the issue: This bmp reports bpp=0, therefore SDL_CalculatePitch()
returns pitch==0, which is then fed to SDL_malloc() (which is malloc())
and malloc(0) returns _something_ which is not NULL but not someting
that we expect.. Then testsprite.c:LoadSprite() accesses the pixels
as *(Uint8*)pixels which valrind reports as:

==15533== Invalid read of size 1
==15533== at 0x8048C08: LoadSprite (testsprite.c:45)
==15533== by 0x80492FC: main (testsprite.c:224)
==15533== Address 0x449e588 is 0 bytes after a block of size 0 alloc'd
==15533== at 0x40072B2: malloc (vg_replace_malloc.c:270)
==15533== by 0x4045719: SDL_CreateRGBSurface (SDL_surface.c:126)
==15533== by 0x40403C1: SDL_LoadBMP_RW (SDL_bmp.c:237)
==15533== by 0x8048BB2: LoadSprite (testsprite.c:36)
==15533== by 0x80492FC: main (testsprite.c:224)

Besides, valrind also reports this:
==15533== Conditional jump or move depends on uninitialised value(s)
==15533== at 0x40403F3: SDL_LoadBMP_RW (SDL_bmp.c:247)
==15533== by 0x8048BB2: LoadSprite (testsprite.c:36)
==15533== by 0x80492FC: main (testsprite.c:224)


Easy/quick solution would be early-rejecting a bmp with 0 bpp from SDL_bmp.c:SDL_LoadBMP_RW()
src/video/SDL_bmp.c
     1.1 --- a/src/video/SDL_bmp.c	Mon Sep 02 12:35:00 2019 +0300
     1.2 +++ b/src/video/SDL_bmp.c	Tue Sep 03 11:55:20 2019 -0700
     1.3 @@ -331,6 +331,7 @@
     1.4          ExpandBMP = biBitCount;
     1.5          biBitCount = 8;
     1.6          break;
     1.7 +    case 0:
     1.8      case 2:
     1.9      case 3:
    1.10      case 5: