Skip to content

Bounds-check macroblock/motion/audio indices to fix four OOB accesses (#73)#74

Open
nakata-app wants to merge 1 commit into
phoboslab:masterfrom
nakata-app:fix/oob-bounds-checks
Open

Bounds-check macroblock/motion/audio indices to fix four OOB accesses (#73)#74
nakata-app wants to merge 1 commit into
phoboslab:masterfrom
nakata-app:fix/oob-bounds-checks

Conversation

@nakata-app

Copy link
Copy Markdown

This fixes the four memory-safety bugs reported in #73, all reachable from crafted MPEG-1 input via plm_decode_video() / plm_decode_audio(). Three targeted bounds checks, no behavior change on valid streams.

1. plm_video_decode_macroblock, negative mb_row/mb_col (Bug A read at :3532, Bug D write at :3514/:3519)

The existing guard checks only the upper bound. When macroblock_address is driven negative, mb_row/mb_col become negative, both >= checks pass, and the derived di index goes negative. In the non-intra path that is an OOB read (d[di]); in the intra path PLM_BLOCK_SET does an OOB write (heap corruption, 16 bytes before the frame plane). Adding the lower-bound check closes both.

2. plm_video_process_macroblock, half-pel interpolation past plane end (Bug B read at :3372)

The old max_address guard reserves room for the block_size x block_size block but not for the half-pel neighbors the interpolation cases read (s[si + dw + 1] at the bottom-right). The new guard checks the actual maximum index touched: the block span for the write side (di), plus the interpolation neighbor (+ dw + 1, gated on odd_h/odd_v) for the read side (si). This rejects only reads that would leave the plane; valid motion compensation stays in-bounds.

3. plm_audio_decode_header, bitrate_index == -1 (Bug C, global read at :4076)

bitrate_index = read(4) - 1 yields -1 for the free-format nibble 0. The guard only rejects > 13, so -1 slips through to PLM_AUDIO_BIT_RATE[-1]. Reject the negative case too.

Verification

- decode_macroblock: reject negative mb_row/mb_col (OOB read at :3532,
  OOB write at :3514/:3519)
- process_macroblock: bound half-pel interpolation reads to the plane (:3372)
- audio_decode_header: reject bitrate_index == -1 (global read at :4076)

No behavior change on valid streams: a 75-frame MPEG-1 clip decodes to
byte-identical output before and after.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant