We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4500 - Heap-Buffer Overflow in Map1toN pertaining to SDL_pixels.c
Summary: Heap-Buffer Overflow in Map1toN pertaining to SDL_pixels.c
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: HG 1.2
Hardware: x86_64 Linux
: P2 critical
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-08 08:55 UTC by Radue
Modified: 2019-02-18 15:52 UTC (History)
1 user (show)

See Also:

Attachments
PoC (578 bytes, image/bmp)
2019-02-08 08:55 UTC, Radue 		Details
Fix (1.67 KB, patch)
2019-02-15 16:10 UTC, Petr Pisar 		Details | Diff
Fix (1.88 KB, patch)
2019-02-18 13:59 UTC, Petr Pisar 		Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Radue 2019-02-08 08:55:05 UTC 
Created attachment 3606 [details]
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
8. ./testsprite PoC
Comment 1 Radue 2019-02-10 14:59:08 UTC 
Assigned CVE-2019-7638 by MITRE.
Comment 2 Petr Pisar 2019-02-15 14:44:35 UTC 
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; i<src->ncolors; ++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.
Comment 3 Petr Pisar 2019-02-15 15:45:13 UTC 
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.
Comment 4 Petr Pisar 2019-02-15 16:10:08 UTC 
Created attachment 3624 [details]
Fix
Comment 5 Petr Pisar 2019-02-18 13:59:05 UTC 
Created attachment 3631 [details]
Fix

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