From 892c8d5058d3bb6edb948724c9756aec9ed57e74 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Tue, 3 Sep 2019 11:55:20 -0700 Subject: [PATCH] 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 file changed, 1 insertion(+) diff --git a/src/video/SDL_bmp.c b/src/video/SDL_bmp.c index 5616f050b83d2..3ee40564c120a 100644 --- a/src/video/SDL_bmp.c +++ b/src/video/SDL_bmp.c @@ -331,6 +331,7 @@ SDL_LoadBMP_RW(SDL_RWops * src, int freesrc) ExpandBMP = biBitCount; biBitCount = 8; break; + case 0: case 2: case 3: case 5: