Two heap out-of-bounds reads in the MPEG-1 video decoder (decode_block :3532, process_macroblock :3372)

[nakata-app]

Summary

Coverage-guided fuzzing (AFL++, ASan) of plm_decode_video() on crafted MPEG-1 input found two distinct heap out-of-bounds reads in the video decoder on current master (pl_mpeg.h SHA-256 3a8cb30c83c2a1147719c30fe0c8b93da2987aa43140077c575b39aaa75fc2c9, the latest commit "Fix corrupt slice check; close #64").

Both are OOB reads (crash / potential info-disclosure class, no write observed), reachable from plm_decode_video() on attacker-controlled input. Neither matches an open issue: #68/#69 are audio-path; #66 is a video correctness bug, not memory safety.

Filing publicly following the precedent of the accepted audio memory-safety reports (#68/#69).

---

Bug A, plm_video_decode_block (pl_mpeg.h:3532)

ASan: READ of size 1, 16 bytes before the frame-plane allocation.

#0 plm_video_decode_block        pl_mpeg.h:3532
#1 plm_video_decode_macroblock   pl_mpeg.h:3231
#2 plm_video_decode_slice        pl_mpeg.h:3130
#3 plm_video_decode_picture      pl_mpeg.h:3094

Line 3532 is the non-intra reconstruction read PLM_BLOCK_SET(d, di, dw, si, 8, 8, plm_clamp(d[di] + s[si])). The destination index is computed straight from mb_row/mb_col with no lower-bound clamp:

di = (self->mb_row * self->luma_width + self->mb_col) << 4;          // luma
di = ((self->mb_row * self->luma_width) << 2) + (self->mb_col << 3);  // chroma

On the crafted stream di underflows to a small negative value, so d[di] reads before the plane. The #64 fix bounds macroblock_address but not this per-block prediction index.

Suggested fix: clamp/validate di to [0, plane_size - block extent] before PLM_BLOCK_SET, or reject when mb_row/mb_col fall outside [0, mb_height) / [0, mb_width).

---

Bug B, plm_video_process_macroblock (pl_mpeg.h:3372)

ASan: READ of size 1, just past the end of the reference-plane allocation.

#0 plm_video_process_macroblock  pl_mpeg.h:3372
#1 plm_video_decode_macroblock   pl_mpeg.h:3221
#2 plm_video_decode_slice        pl_mpeg.h:3130

The bounds check at 3359-3361 validates only si/di:

unsigned int max_address = (dw * (self->mb_height * block_size - block_size + 1) - block_size);
if (si > max_address || di > max_address) { return; }

But the half-pel interpolation cases read up to s[si + dw + 1]:

PLM_MB_CASE(0, 1, 0, (s[si] + s[si + 1] + 1) >> 1);        // line 3372: reads s[si+1]
PLM_MB_CASE(0, 0, 1, (s[si] + s[si + dw] + 1) >> 1);       // reads s[si+dw]
PLM_MB_CASE(0, 1, 1, ... s[si + dw + 1] ...);              // reads s[si+dw+1]

When si is at/near max_address, si + dw + 1 runs off the end of the reference plane. Sibling lines 3371/3373/3376/3378 (the other interpolation cases) share the class.

Suggested fix: guard the actual max index touched, e.g. require si + dw + 1 <= plane_size - 1 (and di + dw + block_size on the write side), not just si <= max_address.

---

Reproduce

Harness reads a stream from stdin and decodes all video+audio frames:

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#define PL_MPEG_IMPLEMENTATION
#include "pl_mpeg.h"

int main(void) {
    size_t cap = 1<<16, len = 0;
    uint8_t *buf = malloc(cap);
    for (;;) {
        if (len == cap) buf = realloc(buf, cap *= 2);
        size_t got = fread(buf + len, 1, cap - len, stdin);
        len += got;
        if (!got) break;
    }
    if (!len) return 0;
    plm_t *plm = plm_create_with_memory(buf, len, 1);
    if (!plm) return 0;
    plm_set_audio_enabled(plm, 1);
    while (plm_decode_video(plm)) {}
    plm_destroy(plm);
    return 0;
}

clang -g -O1 -fsanitize=address -DPL_MPEG_IMPLEMENTATION harness.c -o f
./f < A.mpg   # -> heap-buffer-overflow READ :3532
./f < B.mpg   # -> heap-buffer-overflow READ :3372

Reproducers (286 bytes each, afl-tmin minimized). Decode base64 to reproduce:

A.mpg (sha256 78803129…, triggers Bug A):

AAABuiEwMDAwMDAwAAABuzAwMDAwADAAAAHgAQEAAAABszAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAAAAEAMDAwMDAwMDAwMDAwQjAwQTAwRTAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwATAwMDAwMAgwATAwMDAwMAAAAQEwAUFBMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMA==

B.mpg (sha256 0760bafd…, triggers Bug B):

AAABuicwMDAwMDAwAAABuzAwMDAwADAAAAHgAQEAAAABszAwAAAwRjAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAAAAEAMDAwMDAwMDAwMDAwMDAwMDAwMEMwMDAwMDAwMDAwMDAwMDAwMDAwMDAwATAwMDAwMAgwATAwMDAwMDAAAAEBMKkwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMA==

base64 -d > A.mpg <<< '...'   # paste block above

Severity note

Both are out-of-bounds reads, not writes; no code-execution primitive. Impact is a crash under ASan/hardened builds (DoS) and a theoretical heap read leaking into decoded pixels. Reporting for completeness / hardening.

[nakata-app]

Bug C, global out-of-bounds read in plm_audio_decode_header (pl_mpeg.h:4076)

Same fuzzing campaign surfaced a third, distinct memory-safety bug, this one on the audio path. It is separate from the two video bugs above and from the existing audio issues #68 (plm_audio_decode_frame) and #69 (plm_audio_find_frame_sync); this one is in the audio header parser and reads a global table at index -1.

ASan: READ of size 2, 2 bytes before the global PLM_AUDIO_BIT_RATE (defined at pl_mpeg.h:3680):

#0 plm_audio_decode_header  pl_mpeg.h:4076
#1 plm_audio_decode         pl_mpeg.h:3960
#2 plm_decode_audio         pl_mpeg.h:1219
0x... is located 2 bytes before global variable 'PLM_AUDIO_BIT_RATE' (size 56)

Root cause: signed underflow, lower bound not guarded

// pl_mpeg.h:4028
int bitrate_index = plm_buffer_read(self->buffer, 4) - 1;   // reads 0..15, minus 1 => -1..14
if (bitrate_index > 13) {                                    // signed int; -1 > 13 is false
    return 0;
}
...
// pl_mpeg.h:4076
int bitrate = PLM_AUDIO_BIT_RATE[self->bitrate_index];       // bitrate_index == -1 -> read table[-1]

bitrate_index is a 4-bit field (0..15). A raw value of 0 (MPEG-1 Layer II "free format") becomes -1 after the - 1. The guard only checks the upper bound (> 13), so -1 passes, and line 4076 reads PLM_AUDIO_BIT_RATE[-1] (a signed short[28] global). The raw value 15 is correctly rejected (14 > 13), only the 0 -> -1 underflow slips through.

Suggested fix

Reject the negative case as well, e.g.:

int bitrate_index = plm_buffer_read(self->buffer, 4) - 1;
if (bitrate_index < 0 || bitrate_index > 13) {   // also reject free-format (raw 0) and forbidden (raw 15)
    return 0;
}

Reproduce

The two video reproducers above only exercise plm_decode_video. This bug is on the audio path, so drive plm_decode_audio:

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#define PL_MPEG_IMPLEMENTATION
#include "pl_mpeg.h"

int main(void) {
    static uint8_t buf[1 << 16];
    size_t len = fread(buf, 1, sizeof buf, stdin);
    plm_t *plm = plm_create_with_memory(buf, len, 0);
    if (!plm) return 0;
    for (int i = 0; i < 2000; i++) {
        if (plm_get_num_video_streams(plm) > 0) plm_decode_video(plm);
        if (plm_get_num_audio_streams(plm) > 0) plm_decode_audio(plm);
        if (plm_has_ended(plm)) break;
    }
    plm_destroy(plm);
    return 0;
}

clang -g -O1 -fsanitize=address -DPL_MPEG_IMPLEMENTATION harness_audio.c -o fa
./fa < C.mpg    # -> global-buffer-overflow READ, PLM_AUDIO_BIT_RATE[-1] at :4076

C.mpg (64 bytes, sha256 4133b945…):

AAABuiUBAAEA/wkBAADMCSAAIBMHAeAY4AcAAAG7AAgAwgEXAAAAAAAAAcQAAAHAAAgbAAABE//9BwAAAAABAA==

base64 -d > C.mpg <<< '...'   # paste block above

Severity is the same class as the video bugs: an out-of-bounds read (small, adjacent global), crash under ASan/hardened builds, no write primitive.

[nakata-app]

Severity upgrade for Bug A: the decode_block di underflow is an out-of-bounds heap WRITE, not only a read

My original Bug A described the non-intra read at pl_mpeg.h:3532 (plm_clamp(d[di] + s[si])). Further triage of the same fuzzing corpus shows the underlying di underflow is worse than an info-leak: in the intra branch of plm_video_decode_block, the same negative di drives PLM_BLOCK_SET to write out of bounds, i.e. attacker-influenced heap memory corruption.

Two distinct crashing sites, both heap-buffer-overflow WRITEs 16 bytes before the frame plane (allocated in plm_video_decode_sequence_header:3012):

WRITE of size 1  plm_video_decode_block  pl_mpeg.h:3519   <- intra, IDCT path
WRITE of size 8  plm_video_decode_block  pl_mpeg.h:3514   <- intra, DC-only (n==1) path
  #1 plm_video_decode_macroblock  pl_mpeg.h:3231
  #2 plm_video_decode_slice       pl_mpeg.h:3130
  #3 plm_video_decode_picture     pl_mpeg.h:3094

The intra reconstruction writes straight into d[di] with no read first:

// pl_mpeg.h ~3510, macroblock_intra branch
if (n == 1) {
    int clamped = plm_clamp((s[0] + 128) >> 8);
    PLM_BLOCK_SET(d, di, dw, si, 8, 8, clamped);      // line 3514: WRITE d[di..]
}
else {
    plm_video_idct(s);
    PLM_BLOCK_SET(d, di, dw, si, 8, 8, plm_clamp(s[si]));  // line 3519: WRITE d[di..]
}

di is the same index from Bug A:

di = (self->mb_row * self->luma_width + self->mb_col) << 4;          // luma
di = ((self->mb_row * self->luma_width) << 2) + (self->mb_col << 3);  // chroma

When it underflows negative on a crafted stream, PLM_BLOCK_SET writes an 8x8 block starting before the plane. So the same missing lower-bound clamp yields an OOB write primitive on the intra path and an OOB read on the non-intra path (:3532). The write is the higher-severity case (memory corruption, not just disclosure).

The fix is the same as suggested for Bug A and covers all three sites (3514/3519/3532): validate/clamp di (and reject mb_row/mb_col outside [0, mb_height) / [0, mb_width)) before any PLM_BLOCK_SET.

Reproduce

Same harness as the two video reproducers above (plm_decode_video loop).

W.mpg (333 bytes, sha256 d1105b09…, triggers the :3519 WRITE):

AAABuiEwMDAwMDAwAAABuzAwMDAwATAAAAHgATAAAAABszAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMAAAAQAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDBBMDAwMDAwMDAwMDAwMDAwMAAAAQEwAwAwMDAwMDAwMDAwQUEwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw

base64 -d > W.mpg <<< '...'
./f_plmpeg < W.mpg    # -> heap-buffer-overflow WRITE, pl_mpeg.h:3519

[eobi]

While fuzzing (libFuzzer + ASan) I reproduced both OOB reads reported here and have a verified fix for the plm_video_process_macroblock (:3372) half:

The half-pel interpolation cases read one extra row and/or column (s[si + dw + 1]), but max_address only bounds the base block, so a motion vector that puts si near the plane edge makes those reads run past the plane by up to dw + 1. Tightening the guard by the half-pel extent fixes it:

    unsigned int max_address = (dw * (self->mb_height * block_size - block_size + 1) - block_size);
    unsigned int hp_slack = (odd_v ? dw : 0) + (odd_h ? 1u : 0);
    if (si + hp_slack > max_address || di > max_address) {
        return; // corrupt video
    }

Verified: the process_macroblock reproducer no longer faults, valid video still decodes, and re-fuzzing no longer hits :3372.

The plm_video_decode_block (:3532) half is separate: it writes an 8×8 block at di with no plane-bounds check. The direction is to bound di + 7*dw + 7 against the luma/chroma plane size before the writes, but I don't have a fully-verified patch for that half yet, so I'll leave it to you.

[nakata-app]

Both halves fixed, verified under ASan

Thanks for the independent repro, @eobi — your process_macroblock (:3372) fix is correct and I applied it as-is. I also closed the decode_block (:3532) half, which is the worse one since (as noted above) the di underflow is an out-of-bounds heap write in the intra branch, not just a read.

plm_video_process_macroblock (:3372) — half-pel slack

The half-pel cases read up to s[si + dw + 1] but max_address only bounded the base block. Guard now accounts for the extra row/column:

unsigned int hp_slack = (odd_v ? dw : 0) + (odd_h ? 1u : 0);
if (si + hp_slack > max_address || di > max_address) {
    return; // corrupt video
}

plm_video_decode_block (:3532) — bound the full write extent

mb_row/mb_col can be negative or past the plane edge on corrupt streams (the :3192 guard only checks the upper bound, not the lower). The 8×8 block write spans di .. di + 7*dw + 7, so I bound the whole extent against the plane size before writing:

int plane_size = (block < 4) ? (self->luma_width * self->luma_height)
                             : (self->chroma_width * self->chroma_height);
if (di < 0 || di + 7 * dw + 7 >= plane_size) {
    return; // corrupt video
}

Verification

• Before: A.mpg → heap-buffer-overflow at pl_mpeg.h:3532; B.mpg → heap-buffer-overflow at pl_mpeg.h:3372 (both exit 134 under ASan).
• After: both reproducers decode cleanly, exit 0, no ASan report.
• Valid MPEG-1 (ffmpeg testsrc, 25 frames) still decodes correctly.

Commit: 685af1f on master.