Global Buffer Overflow found within SDL_LoadWAV_RW

[SDLBugzilla]

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• PoC (crash_3_poc.wav, audio/wav, 2019-02-05 15:21:50 +0000, 121946 bytes)
• Fix (file_4492.txt, text/plain, 2019-02-14 13:41:25 +0000, 455 bytes)
• Fix for an invalid chunk length (0001-CVE-2019-7577-Fix-a-buffer-overread-in-MS_ADPCM_deco.patch, text/plain, 2019-02-14 13:42:10 +0000, 2737 bytes)
• Fix for an invalid predictor (SDL-1.2.15-CVE-2019-7577-Fix-a-buffer-overread-in-MS_ADPCM_nibb.patch, text/plain, 2019-03-12 16:02:35 +0000, 1836 bytes)

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

Comments on the original bug report:

On 2019-02-05 15:21:50 +0000, Radue wrote:

Created attachment 3598
PoC

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

Asan output:

=================================================================
==3088==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7fb9bdd13fa4 at pc 0x7fb9bd984c73 bp 0x7ffcf71915d0 sp 0x7ffcf71915c8
READ of size 2 at 0x7fb9bdd13fa4 thread T0
# 0 0x7fb9bd984c72 in SDL_LoadWAV_RW /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/audio/SDL_wave.c:536:8
# 1 0x4db938 in main /home/radu/apps/sdl_player_lib/SDL-1.2.15/test/loopwave.c:76:7
# 2 0x7fb9bc6f682f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
# 3 0x4352f8 in _start (/home/radu/apps/sdl_player_lib/SDL-1.2.15/test/loopwave+0x4352f8)

0x7fb9bdd13fa4 is located 28 bytes to the left of global variable 'SDL_CDcaps' defined in '../src/cdrom/SDL_cdrom.c:37:15' (0x7fb9bdd13fc0) of size 80
0x7fb9bdd13fa4 is located 4 bytes to the right of global variable 'MS_ADPCM_state' defined in '../src/audio/SDL_wave.c:45:3' (0x7fb9bdd13f60) of size 64
SUMMARY: AddressSanitizer: global-buffer-overflow /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/audio/SDL_wave.c:536 SDL_LoadWAV_RW
Shadow bytes around the buggy address:
0x0ff7b7b9a7a0: 00 00 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 f9
0x0ff7b7b9a7b0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
0x0ff7b7b9a7c0: f9 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
0x0ff7b7b9a7d0: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
0x0ff7b7b9a7e0: 00 00 00 00 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
=>0x0ff7b7b9a7f0: 00 00 00 00[f9]f9 f9 f9 00 00 00 00 00 00 00 00
0x0ff7b7b9a800: 00 00 f9 f9 f9 f9 f9 f9 01 f9 f9 f9 f9 f9 f9 f9
0x0ff7b7b9a810: 00 f9 f9 f9 f9 f9 f9 f9 01 f9 f9 f9 f9 f9 f9 f9
0x0ff7b7b9a820: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
0x0ff7b7b9a830: 04 f9 f9 f9 f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9
0x0ff7b7b9a840: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==3088==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. make
8. ./loopwave PoC

On 2019-02-07 07:18:38 +0000, Radue wrote:

Assigned CVE-2019-7577 by MITRE.

On 2019-02-12 17:04:04 +0000, Petr Pisar wrote:

The same issue seems to exists in SDL2 too.

SDL-1.2.15 actually crashes for me on POC WAV file at

MS_ADPCM_decode()
[...]
/* Decode and store the other samples in this block /
samplesleft = (MS_ADPCM_state.wSamplesPerBlock-2)
MS_ADPCM_state.wavefmt.channels;
while ( samplesleft > 0 ) {
---> nybble = (*encoded)>>4;
new_sample = MS_ADPCM_nibble(state[0],nybble,coeff[0]);
decoded[0] = new_sample&0xFF;
new_sample >>= 8;
decoded[1] = new_sample&0xFF;
decoded += 2;

                    nybble = (*encoded)&0x0F;
                    new_sample = MS_ADPCM_nibble(state[1],nybble,coeff[1]);
                    decoded[0] = new_sample&0xFF;
                    new_sample >>= 8;
                    decoded[1] = new_sample&0xFF;
                    decoded += 2;

                    ++encoded;
                    samplesleft -= 2;
            }

with this back trace:

(gdb) bt full

0 0x00007ffff7f159ba in MS_ADPCM_decode (audio_buf=0x404100 <wave+32>, audio_len=0x404108 <wave+40>) at ./src/au

dio/SDL_wave.c:191
state = {0x7ffff7f9c370 <MS_ADPCM_state+48>, 0x7ffff7f9c370 <MS_ADPCM_state+48>}
freeable = 0x46b940 ""
encoded = 0x4aa000 <error: Cannot access memory at address 0x4aa000>
decoded = 0x7ffff31c1ab0 ""
encoded_len = 120320
samplesleft = 9856
nybble = 0 '\000'
stereo = 0 '\000'
coeff = {0x7ffff7f9c354 <MS_ADPCM_state+20>, 0x7ffff7f9c354 <MS_ADPCM_state+20>}
new_sample = 0

1 0x00007ffff7f16464 in SDL_LoadWAV_RW (src=0x460280, freesrc=1, spec=0x4040e0 , audio

_buf=0x404100 <wave+32>, audio_len=0x404108 <wave+40>) at ./src/audio/SDL_wave.c:536
    was_error = 0
    chunk = {magic = 1635017060, length = 121856, data = 0x46b940 ""}
    lenread = 121856
    MS_ADPCM_encoded = 1
    IMA_ADPCM_encoded = 0
    samplesize = 0
    RIFFchunk = 1179011410
    wavelen = 121938
    WAVEmagic = 1163280727
    headerDiff = 82
    format = 0x4487c0

2 0x00000000004013a7 in main (argc=2, argv=0x7fffffffe1b8) at loopwave.c:76

    name = "\360\024@\000\000\000\000\000@\021@\000\000\000\000\000\260\341\377\377\377\177\000\000\000\000\000\000\000\000\000"

The issue is that the POC file defines a bogus number of channels (129), blockalign (512) and maybe others:

(gdb) print MS_ADPCM_state.wavefmt
$4 = {encoding = 2, channels = 129, frequency = 22050, byterate = 11155, blockalign = 512,
bitspersample = 4}

These values are taken directly from the file and based on them the samplesleft value (number of bytes available in the output buffer) is computed. However this value has no relationship to encoded pointer that advances through the input buffer until the while() loop terminates (samplesleft > 0). The input buffer length is driven by an independent headers in the WAV stream. The loop advances pointer to the input buffer by a byte and output buffer by two bytes in each iteration.

As a result the while() loop runs out of the input buffer sooner than the output buffer is filled with decoded samples. And that results into reading from an uninitialized (and in my case unallocated) memory and that causes the crash.

In addition, an outer while-loop() blindly advances encoded pointer without checking enough data were read (encoded_len). And it also assumes only monophonic or sterophonic sound (compare to 129).

I believe that a proper fix should restrict the decoder to 1 <= channels <= 2 and checking encoded pointer does fall behind encoded_len. I also believe that similar issue exist in IMA_ADPCM_decode() function.

On 2019-02-14 13:41:25 +0000, Petr Pisar wrote:

Created attachment 3607
Fix

On 2019-02-14 13:42:10 +0000, Petr Pisar wrote:

Created attachment 3608
Fix for an invalid chunk length

This patch fixes the crash I saw in MS_ADPCM_decode() with a reproducer attached in this bug report.

First I thought about some complex formula that checks a chunk has a minimal length safe for decoding it. But nesting RIFF chunks of various types makes it difficult and it effectively would split the logic into two distant places, so I decided to use multiple local fixes -- to check a a current parsing code won't read past the current chunk buffer.

On 2019-03-08 02:45:50 +0000, Abhijith wrote:

Hello I was able to reproduce CVE-2019-7577 after applying all the patches from CVE-2019-7572 to CVE-2019-7578. Rest all look good.

./loopwave ../../../Downloads/crash_3_poc.wav

==1625==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7ff48c3bbec4 at pc 0x7ff48c091053 bp 0x7ffe54a69110 sp 0x7ffe54a69108
READ of size 2 at 0x7ff48c3bbec4 thread T0
# 0 0x7ff48c091052 (/usr/lib/x86_64-linux-gnu/libSDL-1.2.so.0+0x2e052)
# 1 0x7ff48c092f85 in SDL_LoadWAV_RW (/usr/lib/x86_64-linux-gnu/libSDL-1.2.so.0+0x2ff85)
# 2 0x401384 in main (/home/abhijith/ssdr/libsdl1.2-1.2.15/test/loopwave+0x401384)
# 3 0x7ff48bcd9b44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
# 4 0x400e58 (/home/abhijith/ssdr/libsdl1.2-1.2.15/test/loopwave+0x400e58)

0x7ff48c3bbec4 is located 60 bytes to the left of global variable 'SDL_CDcaps' from './src/cdrom/SDL_cdrom.c' (0x7ff48c3bbf00) of size 80
0x7ff48c3bbec4 is located 4 bytes to the right of global variable 'MS_ADPCM_state' from './src/audio/SDL_wave.c' (0x7ff48c3bbe80) of size 64
SUMMARY: AddressSanitizer: global-buffer-overflow ??:0 ??
Shadow bytes around the buggy address:
0x0fff1186f780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0fff1186f790: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0fff1186f7a0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
0x0fff1186f7b0: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
0x0fff1186f7c0: 00 00 00 00 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
=>0x0fff1186f7d0: 00 00 00 00 00 00 00 00[f9]f9 f9 f9 00 00 00 00
0x0fff1186f7e0: 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9
0x0fff1186f7f0: 00 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
0x0fff1186f800: 01 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
0x0fff1186f810: 00 f9 f9 f9 f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9
0x0fff1186f820: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
Contiguous container OOB:fc
ASan internal: fe
==1625==ABORTING

On 2019-03-12 15:11:41 +0000, Petr Pisar wrote:

Thank you for verifying the patches. You are right, the reproducer triggers another issue valgrind I used was unable to detect.

The another issue is that the 130291th sample has stored invalid predictors. An acceptable value is between 0 and 6 inclusive, but the stored predictors have 12 value. Because the predictor is used as an index into an array of coefficients, we got a read past the MS_ADPCM_state.aCoeff[] array boundary in MS_ADPCM_nibble() when dereferencing coeff pointer:

0 MS_ADPCM_nibble (state=0x7ffff75582f0 <MS_ADPCM_state+48>, nybble=3 '\003',

coeff=0x7ffff7558304) at ./src/audio/SDL_wave.c:99

99 new_sample = ((state->iSamp1 * coeff[0]) +
(gdb) c
Continuing.

==25913==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7ffff7558304 at pc 0x7ffff73c08be bp 0x7fffffffdde0 sp 0x7fffffffddd0
READ of size 2 at 0x7ffff7558304 thread T0
# 0 0x7ffff73c08bd in MS_ADPCM_nibble src/audio/SDL_wave.c:99
# 1 0x7ffff73c1bf7 in MS_ADPCM_decode src/audio/SDL_wave.c:206
# 2 0x7ffff73c3e42 in SDL_LoadWAV_RW src/audio/SDL_wave.c:573
# 3 0x401525 in main /tmp/SDL-1.2.15/test/loopwave.c:76
# 4 0x7ffff71e5f72 in __libc_start_main (/lib64/libc.so.6+0x26f72)
# 5 0x4011ed in _start (/tmp/SDL-1.2.15/test/a.out+0x4011ed)

We need to validate the predictors.

On 2019-03-12 16:02:35 +0000, Petr Pisar wrote:

Created attachment 3694
Fix for an invalid predictor

This fixes the issue with an invalid predictor.

On 2019-06-10 15:55:23 +0000, Sam Lantinga wrote:

Fixed, thanks!
https://hg.libsdl.org/SDL/rev/faf9abbcfb5f
https://hg.libsdl.org/SDL/rev/416136310b88

This code has been rewritten in SDL 2.0, can you verify that this issue has been fixed there?
http://www.libsdl.org/tmp/SDL-2.0.zip

On 2019-06-10 21:00:28 +0000, Simon Hug wrote:

The WAVE file (attachment 3598) specifies 129 channels for an MS ADPCM format.

With the current tip, SDL_LoadWAV_RW rejects this file with "Invalid number of channels" as it only supports mono and stereo for MS ADPCM (the specification does not go beyond that anyway).

On 2019-06-11 13:23:52 +0000, Sam Lantinga wrote:

Great, thanks!