Heap-Buffer Overflow in Map1toN pertaining to SDL_pixels.c

[SDLBugzilla]

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• PoC (crash_2_video_poc.bmp, image/bmp, 2019-02-08 08:55:05 +0000, 578 bytes)
• Fix (0001-CVE-2019-7638-Refuse-loading-BMP-images-with-too-hig.patch, text/plain, 2019-02-15 16:10:08 +0000, 1708 bytes)
• Fix (0001-CVE-2019-7638-CVE-2019-7636-Refuse-loading-BMP-image.patch, text/plain, 2019-02-18 13:59:05 +0000, 1922 bytes)

Reported in version: HG 1.2
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2019-02-08 08:55:05 +0000, Radue wrote:

Created attachment 3606
PoC

A heap-buffer overflow vulnerability was discovered in SDL-1.2.15 library.

ASAN report:

=================================================================
==30184==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61900000e580 at pc 0x7f17151f9538 bp 0x7ffe8733fb30 sp 0x7ffe8733fb28
READ of size 1 at 0x61900000e580 thread T0
# 0 0x7f17151f9537 in Map1toN /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_pixels.c:482:3
# 1 0x7f17151f9537 in SDL_MapSurface /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_pixels.c:583
# 2 0x7f1715204deb in SDL_LowerBlit /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_surface.c:417:8
# 3 0x7f1715204deb in SDL_ConvertSurface /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_surface.c:866
# 4 0x7f171520aa40 in SDL_DisplayFormat /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_video.c:946:9
# 5 0x4dba11 in LoadSprite /home/radu/apps/sdl_player_lib/SDL-1.2.15/test/testsprite.c:41:9
# 6 0x4dbc8f in main /home/radu/apps/sdl_player_lib/SDL-1.2.15/test/testsprite.c:98:7
# 7 0x7f1713ede82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
# 8 0x435528 in _start (/home/radu/apps/sdl_player_lib/SDL-1.2.15/test/testsprite+0x435528)

0x61900000e580 is located 0 bytes to the right of 1024-byte region [0x61900000e180,0x61900000e580)
allocated by thread T0 here:
# 0 0x4bc4f2 in malloc (/home/radu/apps/sdl_player_lib/SDL-1.2.15/test/testsprite+0x4bc4f2)
# 1 0x7f17151f4ae8 in SDL_AllocFormat /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_pixels.c:133:44

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_pixels.c:482 Map1toN
Shadow bytes around the buggy address:
0x0c327fff9c60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c327fff9c70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c327fff9c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c327fff9c90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c327fff9ca0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c327fff9cb0:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c327fff9cc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c327fff9cd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c327fff9ce0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c327fff9cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c327fff9d00: 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 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
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==30184==ABORTING

Poc: See attachment
Reproducing steps:

1. Download SDL-1.2.15 library
2. ./configure with Asan enabled
3. ./make
4. sudo make install
5. cd examples
6. ./configure with Asan enabled
7. modify testsprite.c to load the poc.bmp file
8. make
9. ./testsprite PoC

On 2019-02-10 14:59:08 +0000, Radue wrote:

Assigned CVE-2019-7638 by MITRE.

On 2019-02-15 14:44:35 +0000, Petr Pisar wrote:

My valgrind output actually points to:

==29868== Invalid read of size 1
==29868== at 0x4882F41: Map1to1 (SDL_pixels.c:459)
==29868== by 0x48835AD: SDL_MapSurface (SDL_pixels.c:569)
==29868== by 0x4884978: SDL_LowerBlit (SDL_surface.c:417)
==29868== by 0x48859AA: SDL_ConvertSurface (SDL_surface.c:866)
==29868== by 0x4887EE3: SDL_DisplayFormat (SDL_video.c:946)
==29868== by 0x4013E2: LoadSprite (testsprite.c:49)
==29868== by 0x401B25: main (testsprite.c:224)

And Map1to1() contains:

static Uint8 *Map1to1(SDL_Palette *src, SDL_Palette *dst, int *identical)
{
[...]
for ( i=0; incolors; ++i ) {
map[i] = SDL_FindColor(dst,
---> src->colors[i].r, src->colors[i].g, src->colors[i].b);
}
[...]
}

The issue is that src->colors[i] points to an invalid memory because src->ncolors is ridiculously large (131075) and is much larger than number of elements in src->colors[] array. The SDL_Pallete src comes unchanged from LoadSprite() in testsprite.c program:

int LoadSprite(char *file)
{
[...]
sprite = SDL_LoadBMP(file);
[...]
}

as sprite->format->palette:

(gdb) p sprite->format->palette->ncolors
$20 = 131075
(gdb) p sprite->format->palette->colors[131075-1]
Cannot access memory at address 0x4887f8

This I conclude that SDL_LoadBMP() misparses the crafted BMP file. It's not an issue in SDL video system at all.

On 2019-02-15 15:45:13 +0000, Petr Pisar wrote:

The reproducer has these data in BITMAPINFOHEADER:

biSize = 40
biBitCount = 8
biClrUsed = 131075

SDL_LoadBMP_RW() function passes biBitCount as a color depth to SDL_CreateRGBSurface(), thus 256-color pallete is allocated. But then biClrUsed colors are read from a file and stored into the palette. SDL_LoadBMP_RW should report an error if biClrUsed is greater than 2^biBitCount.

On 2019-02-15 16:10:08 +0000, Petr Pisar wrote:

Created attachment 3624
Fix

On 2019-02-18 13:59:05 +0000, Petr Pisar wrote:

Created attachment 3631
Fix

The fix also resolved CVE-2019-7636 reported in bug # 4499.

On 2019-02-18 15:52:40 +0000, Sam Lantinga wrote:

Fixed, thanks!
https://hg.libsdl.org/SDL/rev/19d8c3b9c251
https://hg.libsdl.org/SDL/rev/07c39cbbeacf