Heap buffer overflow in plm_audio_find_frame_sync (unsigned underflow when length==0)

[ByamB4]

Description

plm_audio_find_frame_sync() contains an unsigned integer underflow that causes a heap buffer overflow (out-of-bounds read). When the internal audio buffer has length == 0, the loop condition i < self->buffer->length - 1 wraps length - 1 to SIZE_MAX, causing the function to iterate far past the allocated buffer.

This was found via coverage-guided fuzzing (libFuzzer + AddressSanitizer) and confirmed with a standalone ASAN binary.

Root Cause

In pl_mpeg.h line ~3982:

int plm_audio_find_frame_sync(plm_audio_t *self) {
    size_t i;
    for (i = self->buffer->bit_index >> 3; i < self->buffer->length-1; i++) {
        if (
            self->buffer->bytes[i] == 0xFF &&
            (self->buffer->bytes[i+1] & 0xFE) == 0xFC
        ) {
            self->buffer->bit_index = ((i+1) << 3) + 3;
            return TRUE;
        }
    }
    self->buffer->bit_index = (i + 1) << 3;
    return FALSE;
}

self->buffer->length is size_t (unsigned). When length == 0, the expression length - 1 wraps to SIZE_MAX (~18 quintillion on 64-bit), and the loop reads through and past the entire heap allocation.

Additionally, this function bypasses the normal plm_buffer_has()/plm_buffer_read() bounds-checking API by directly accessing self->buffer->bytes[], which also means it can't trigger load callbacks for ring buffers.

Trigger path:

1. plm_audio_create_with_buffer() calls plm_audio_decode_header()
2. plm_buffer_has(buffer, 48) triggers the load callback, filling the audio ring buffer
3. plm_buffer_skip_bytes(buffer, 0x00) consumes all zero-value bytes, repeatedly triggering load callbacks
4. The demuxer is exhausted; plm_buffer_discard_read_bytes() resets the buffer to length=0, bit_index=0
5. plm_buffer_read(buffer, 11) fails → returns 0 → sync mismatch
6. plm_audio_find_frame_sync() is called with length==0
7. Loop condition i < 0 - 1 becomes i < SIZE_MAX → reads past the 128KB buffer

ASAN Output

==7==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fa3493f2800
READ of size 1 at 0x7fa3493f2800 thread T0
    #0 plm_audio_find_frame_sync /build/pl_mpeg/pl_mpeg.h:3986:4
    #1 plm_audio_decode_header /build/pl_mpeg/pl_mpeg.h:4013:39
    #2 plm_audio_create_with_buffer /build/pl_mpeg/pl_mpeg.h:3906:31
    #3 plm_init_decoders /build/pl_mpeg/pl_mpeg.h:946:26

0x7fa3493f2800 is located 0 bytes to the right of 131072-byte region
[0x7fa3493d2800,0x7fa3493f2800)

Reproduction

// plmpeg_driver.c - Standalone ASAN test driver
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define PL_MPEG_IMPLEMENTATION
#include "pl_mpeg.h"

int main(int argc, char **argv) {
    if (argc < 2) { fprintf(stderr, "Usage: %s <file.mpg>\n", argv[0]); return 1; }
    FILE *f = fopen(argv[1], "rb");
    if (!f) { fprintf(stderr, "Cannot open %s\n", argv[1]); return 1; }
    fseek(f, 0, SEEK_END);
    long fsize = ftell(f);
    fseek(f, 0, SEEK_SET);
    uint8_t *data = (uint8_t *)malloc(fsize);
    fread(data, 1, fsize, f);
    fclose(f);
    plm_t *plm = plm_create_with_memory(data, fsize, TRUE);
    if (!plm) { free(data); return 1; }
    plm_set_audio_enabled(plm, FALSE);
    for (int i = 0; i < 10; i++) {
        plm_frame_t *frame = plm_decode_video(plm);
        if (!frame) break;
    }
    plm_destroy(plm);
    return 0;
}

git clone https://github.com/phoboslab/pl_mpeg.git
clang -g -O1 -fsanitize=address,undefined -fno-sanitize=function \
    -fno-omit-frame-pointer -I pl_mpeg -o plmpeg_asan plmpeg_driver.c
./plmpeg_asan crash.mpg  # 405-byte crafted file triggers the crash

The crash file (405 bytes) was found by libFuzzer within seconds of fuzzing. I can provide the file on request.

Suggested Fix

Add a guard for the empty buffer case:

int plm_audio_find_frame_sync(plm_audio_t *self) {
    size_t i;
    if (self->buffer->length < 2) {
        return FALSE;
    }
    for (i = self->buffer->bit_index >> 3; i < self->buffer->length - 1; i++) {
        if (
            self->buffer->bytes[i] == 0xFF &&
            (self->buffer->bytes[i+1] & 0xFE) == 0xFC
        ) {
            self->buffer->bit_index = ((i+1) << 3) + 3;
            return TRUE;
        }
    }
    self->buffer->bit_index = (i + 1) << 3;
    return FALSE;
}

Additional Issues Found During Audit

1. Heap over-read in plm_video_process_macroblock (line ~3345)

The max_address bounds check doesn't account for half-pixel motion compensation accesses (s[si+1], s[si+dw], s[si+dw+1]). When odd_h=1 or odd_v=1, reads extend up to dw bytes past the plane allocation.

2. Undefined behavior in plm_demux_decode_time (line ~2216)

int64_t clock = plm_buffer_read(self->buffer, 3) << 30;

plm_buffer_read returns int (32-bit). Left-shifting a 3-bit value by 30 can overflow signed 32-bit int before the implicit promotion to int64_t. Fix: cast to int64_t before shifting.

Impact

• Availability: Process crash when processing crafted MPEG files (DoS)
• Confidentiality: Heap contents can be leaked through the over-read

pl_mpeg is widely used as an embedded MPEG decoder in games, media players, and web applications (via Emscripten/WASM). Any application that decodes untrusted MPEG files is affected.

Note: This is distinct from #68 which is about bitrate_index validation in plm_audio_decode_header.