MS ADPCM heap-buffer-overflow still reachable after c48e4c6 with valid `numCoefficients=7`

[mangowithegg]

I believe the previous fix in commit c48e4c6503f7dabd41f11d4c9c7b7f8960e7f2c0 is not fully sufficient for the underlying MS ADPCM heap-buffer-overflow.

That commit adds a runtime check for:

if (numCoefficients < 7 || numCoefficients > 255)
    return AF_FAIL;

This blocks the original PoC where numCoefficients == 0. However, I was able to construct a modified WAV file with a valid numCoefficients == 7 and a complete 7-pair MS ADPCM coefficient table. This file passes the patched check but still triggers a heap-buffer-overflow in MSADPCM::decodeBlock().

Environment

Tested with an ASan/UBSan build of audiofile after manually applying commit:

c48e4c6

Build commands:

CC=clang-10 CXX=clang++-10 ./autogen.sh --disable-docs CFLAGS="-fsanitize=address -fno-omit-frame-pointer -g" CXXFLAGS="-fsanitize=address -fno-omit-frame-pointer -g" LDFLAGS="-fsanitize=address"
make

poc_file:exploit-6832-numcoef7-real.wav

Run command:

BASE=$PWD
POC=/tmp/exploit-6832-numcoef7-real.wav

ASAN_OPTIONS=detect_leaks=0:halt_on_error=1:abort_on_error=1:symbolize=1 \
UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1:abort_on_error=1 \
LD_LIBRARY_PATH="$BASE/libaudiofile/.libs:$BASE/sfcommands/.libs:$BASE/.libs:$LD_LIBRARY_PATH" \
./sfcommands/.libs/sfconvert "$POC" /tmp/out.aiff

ASan Report

==1982553==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62d00001c45a at pc 0x7ffff7b758fc bp 0x7fffffffc9f0 sp 0x7fffffffc9e8
WRITE of size 2 at 0x62d00001c45a thread T0
    #0 0x7ffff7b758fb in MSADPCM::decodeBlock(unsigned char const*, short*) /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/libaudiofile/modules/MSADPCM.cpp:222:14
    #1 0x7ffff7b4ccfa in BlockCodec::runPull() /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/libaudiofile/modules/BlockCodec.cpp:55:3
    #2 0x7ffff7b5ab47 in Module::pull(unsigned long) /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/libaudiofile/modules/Module.cpp:71:12
    #3 0x7ffff7b7bef1 in RebufferModule::runPull() /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/libaudiofile/modules/RebufferModule.cpp:122:3
    #4 0x7ffff7b39238 in afReadFrames /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/libaudiofile/data.cpp:222:14
    #5 0x4c50b7 in copyaudiodata /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/sfcommands/sfconvert.c:340:29
    #6 0x4c4c04 in main /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/sfcommands/sfconvert.c:248:17
    #7 0x7ffff6b07c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310
    #8 0x41b889 in _start (/SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/sfcommands/.libs/sfconvert+0x41b889)

0x62d00001c45a is located 0 bytes to the right of 32858-byte region [0x62d000014400,0x62d00001c45a)
allocated by thread T0 here:
    #0 0x493fcd in malloc (/SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/sfcommands/.libs/sfconvert+0x493fcd)
    #1 0x7ffff6579297 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x93297)
    #2 0x7ffff7b5c7f2 in ModuleState::setup(_AFfilehandle*, Track*) /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/libaudiofile/modules/ModuleState.cpp:174:16
    #3 0x7ffff7b3eea4 in afGetFrameCount /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/libaudiofile/format.cpp:205:41
    #4 0x4c5061 in copyaudiodata /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/sfcommands/sfconvert.c:329:29
    #5 0x4c4c04 in main /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/sfcommands/sfconvert.c:248:17
    #6 0x7ffff6b07c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: heap-buffer-overflow /SoK-main/RCA-baseline/experiments/vul4c/SmartGDB-LLM/audio_source/libaudiofile/modules/MSADPCM.cpp:222:14 in MSADPCM::decodeBlock(unsigned char const*, short*)

Why The Previous Fix Appears Incomplete

The previous fix rejects invalid numCoefficients, which is useful and does block the original input. However, the final out-of-bounds write is not directly caused by numCoefficients == 0.

With the modified PoC, runtime values are:

m_numCoefficients = 7
m_framesPerPacket = 16429
channelCount = 1
outputLength = 32858
samplesRemaining = 16427

The output buffer size is:

16429 * 1 * sizeof(int16_t) = 32858 bytes

But MSADPCM::decodeBlock() computes:

samplesRemaining = (m_framesPerPacket - 2) * m_track->f.channelCount;

which becomes:

(16429 - 2) * 1 = 16427

Then the loop checks only:

while (samplesRemaining > 0)

but each iteration always writes two samples:

*decoded++ = newSample;
...
*decoded++ = newSample;
samplesRemaining -= 2;

When samplesRemaining == 1, the loop still performs two int16_t writes. The first write fills the last available sample slot, and the second write goes 2 bytes past the end of the output buffer.

So the remaining issue seems to be that malformed or inconsistent MS ADPCM packet geometry, especially samplesPerBlock, can still drive the decoder into an odd-residual state that the two-sample loop does not handle safely.

Suggested Direction

The fix may need to additionally validate MS ADPCM packet geometry, not only numCoefficients.

For example, the parser or decoder should reject configurations where samplesPerBlock, blockAlign, and channelCount are inconsistent with the number of samples the decoder loop can safely produce. Alternatively, MSADPCM::decodeBlock() should avoid writing two samples when only one output sample remains.