Skip to content

Conversation

@cfsmp3
Copy link
Contributor

@cfsmp3 cfsmp3 commented Jan 10, 2026

Summary

Follow-up to #1996. Fixes incorrect behavior where a "moov" box was accepted without verifying it contains "mvhd" when the buffer was too small.

The Problem

PR #1996 fixed a panic but introduced a logic error:

// Before #1996 - would panic if buffer too small
if idx == 2 && !(bytes == "mvhd") { continue; }

// After #1996 - accepts moov without verification if buffer too small!
if idx == 2 && position + 15 < buffer.len() && !(bytes == "mvhd") { continue; }

With the #1996 fix, if position + 15 >= buffer.len():

  • The condition short-circuits
  • The box is NOT skipped
  • An unverified "moov" is accepted as valid

The Fix

if idx == 2 {
    if position + 16 > buffer.len() {
        continue;  // Can't verify mvhd - skip this box
    }
    if !(bytes == "mvhd") {
        continue;  // No mvhd - skip this box
    }
}

Now:

  • If buffer too small → skip (safe)
  • If moov has mvhd → accept (valid)
  • If moov lacks mvhd → skip (invalid)

Why This Is Safe

This code runs in detect_stream_type - a one-shot format probe that reads up to 1MB. For format detection:

  1. Skipping an unverifiable box is safer than accepting it
  2. The scoring system requires multiple valid boxes to claim MP4
  3. Real MP4 files have moov+mvhd well within the first 1MB

🤖 Generated with Claude Code

The previous fix (#1996) prevented a panic when the buffer was too small
to verify if a "moov" box contains "mvhd", but it incorrectly accepted
the box without verification.

The original intent was: "moov without mvhd is invalid, skip it."

This fix maintains that intent:
- If buffer too small to verify mvhd → skip the box
- If moov has mvhd → accept (valid)
- If moov lacks mvhd → skip (invalid)

This is safe for format detection since:
1. The probe reads up to 1MB of start bytes
2. The scoring system requires multiple valid boxes
3. Skipping an unverifiable box is safer than accepting it

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit a199f4f...:
Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 6/7
DVD 3/3
DVR-MS 2/2
General 25/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 81/86
Teletext 21/21
WTV 13/13
XDS 34/34

NOTE: The following tests have been failing on the master branch as well as the PR:

  • ccextractor --autoprogram --out=srt --latin1 --quant 0 85271be4d2..., Last passed:

    Test 7743

  • ccextractor --autoprogram --out=ttxt --latin1 --ucla dab1c1bd65..., Last passed:

    Test 7743

  • ccextractor --out=srt --latin1 --autoprogram 29e5ffd34b..., Last passed:

    Test 7743

  • ccextractor --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743

  • ccextractor --startcreditsnotbefore 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743

  • ccextractor --startcreditsnotafter 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743

  • ccextractor --startcreditsforatleast 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743

  • ccextractor --startcreditsforatmost 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed:

    Test 7743


This PR does not introduce any new test failures. However, some tests are failing on both master and this PR (see above).

Check the result page for more info.

@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit b39f923...:
Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 6/7
DVD 3/3
DVR-MS 2/2
General 27/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 85/86
Teletext 21/21
WTV 13/13
XDS 34/34

NOTE: The following tests have been failing on the master branch as well as the PR:


This PR does not introduce any new test failures. However, some tests are failing on both master and this PR (see above).

Check the result page for more info.

@cfsmp3 cfsmp3 merged commit 0228fbc into master Jan 11, 2026
41 checks passed
@cfsmp3 cfsmp3 deleted the fix/mp4-moov-mvhd-bounds-check branch January 11, 2026 09:30
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.

3 participants