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 4522 - Buffer Overflow found within IMA_ADPCM_decode pertaining to SDL_wave.c
Summary: Buffer Overflow found within IMA_ADPCM_decode pertaining to SDL_wave.c
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: audio (show other bugs)
Version: 2.0.9
Hardware: x86_64 Linux
: P2 critical
Assignee: Simon Hug
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-25 10:17 UTC by Quang Nguyen
Modified: 2019-08-21 21:38 UTC (History)
3 users (show)

See Also:

Attachments
Crash (80 bytes, audio/x-wav)
2019-02-25 10:17 UTC, Quang Nguyen 		Details
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 Quang Nguyen 2019-02-25 10:17:29 UTC 
Created attachment 3658 [details]
Crash

The function is defined at line 393 in src/audio/SDL_wave.c:

An exploitable integer overflow vulnerability exists when opening a crafted WAV file in SDL 2.0.9. A specially crafted file can cause a buffer overflow resulting in too little memory being allocated which can lead to the reading out of the supplied input buffer. An attacker can provide a specially crafted wav file to trigger this vulnerability.

static int
IMA_ADPCM_decode(Uint8 ** audio_buf, Uint32 * audio_len)
{
    ////// .....................
	    /* Decode and store the other samples in this block */
        samplesleft = (IMA_ADPCM_state.wSamplesPerBlock - 1) * channels;
        while (samplesleft > 0) {
            for (c = 0; c < channels; ++c) {
                Fill_IMA_ADPCM_block(decoded, encoded,
                                     c, channels, &state[c]);
                encoded += 4;   // vulnerability here, no-check
                samplesleft -= 8;
            }
            decoded += (channels * 8 * 2);
        }
        encoded_len -= IMA_ADPCM_state.wavefmt.blockalign;
    }
    ///////////
}


asan report
```
=================================================================
==6886==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000779 at pc 0x7fe9ea628b7a bp 0x7ffcc9ec5b30 sp 0x7ffcc9ec5b28
READ of size 1 at 0x602000000779 thread T0
    #0 0x7fe9ea628b79 in Fill_IMA_ADPCM_block /mnt/data2/fuzz/sdl_proj/SDL/src/audio/SDL_wave.c:319:19
    #1 0x7fe9ea628b79 in IMA_ADPCM_decode /mnt/data2/fuzz/sdl_proj/SDL/src/audio/SDL_wave.c:393
    #2 0x7fe9ea628b79 in SDL_LoadWAV_RW_REAL /mnt/data2/fuzz/sdl_proj/SDL/src/audio/SDL_wave.c:635
    #3 0x52d9b9 in main /mnt/data2/fuzz/sdl_proj/harness_sdl_wave.c:31:8
    #4 0x7fe9e936a82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #5 0x41ae08 in _start (/mnt/data2/fuzz/sdl_proj/fuzz/harness_sdl+0x41ae08)

0x602000000779 is located 0 bytes to the right of 9-byte region [0x602000000770,0x602000000779)
allocated by thread T0 here:
    #0 0x4eceaf in malloc /mnt/data2/clang+llvm/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146
    #1 0x7fe9ea717614 in SDL_malloc_REAL /mnt/data2/fuzz/sdl_proj/SDL/src/stdlib/SDL_malloc.c:5387:11
    #2 0x52d9b9 in main /mnt/data2/fuzz/sdl_proj/harness_sdl_wave.c:31:8

SUMMARY: AddressSanitizer: heap-buffer-overflow /mnt/data2/fuzz/sdl_proj/SDL/src/audio/SDL_wave.c:319:19 in Fill_IMA_ADPCM_block
Shadow bytes around the buggy address:
  0x0c047fff8090: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff80a0: fa fa fd fd fa fa fd fa fa fa fd fd fa fa fd fd
  0x0c047fff80b0: fa fa fd fa fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff80c0: fa fa fd fa fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff80d0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
=>0x0c047fff80e0: fa fa fd fd fa fa fd fa fa fa 04 fa fa fa 00[01]
  0x0c047fff80f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8130: 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
  Shadow gap:              cc
==6886==ABORTING
```
Comment 1 Sam Lantinga 2019-06-10 16:00:51 UTC 
Simon, can you verify that your changes fix this issue?
Comment 2 Simon Hug 2019-06-10 21:02:54 UTC 
The WAVE file (attachment 3658 [details]) specifies 24 bits per sample for IMA ADPCM.

With the current tip, SDL_LoadWAV_RW rejects this file with "Invalid IMA ADPCM bits per sample of 24" as it only supports 4-bit IMA ADPCM.
Comment 3 Sam Lantinga 2019-06-11 13:27:13 UTC 
Great, thanks!
Comment 4 Anselmo L. S. Melo 2019-07-23 23:43:11 UTC 
This was assigned CVE-2019-13626.

I see Simon's comment this was already fixed with the tip on 2019-06-10. Which commit fixed this issue? Thanks.
Comment 5 Hugo Lefeuvre 2019-08-09 13:34:23 UTC 
This issue was probably fixed by the new sanity checks introduced in
https://hg.libsdl.org/SDL/rev/b06fa7da012b

Can anybody confirm? Thanks!
Comment 6 Mike Gorse 2019-08-21 21:38:11 UTC 
I tried compiling loopwave.c, running
hg update 12805
and testing with the reproducer. I got a segfault. I updated to 12806 and ran the same test and saw an error, rather than a crash. So that (large) commit has the fix.