Skip to content

fix(Mp42Hls): reject E-AC-3 sample descriptions with no usable dec3 atom - #1098

Open
AetherAI3 wants to merge 1 commit into
axiomatic-systems:masterfrom
AetherAI3:fix/1043-mp42hls-null-dec3-atom
Open

AetherAI3 wants to merge 1 commit into
axiomatic-systems:masterfrom
AetherAI3:fix/1043-mp42hls-null-dec3-atom

Conversation

@AetherAI3

Copy link
Copy Markdown

Closes #1043.

The E-AC-3 branch of mp42hls's audio-stream setup does

AP4_Dec3Atom* dec3 = AP4_DYNAMIC_CAST(AP4_Dec3Atom,
    sample_description->GetDetails().GetChild(AP4_ATOM_TYPE_DEC3));
...
if (dec3->GetSubStreams()[0].acmod == 0) { ... }

with no check that the cast returned non-NULL, and no check that the substreams array is non-empty. A crafted MP4 whose sample description advertises EC-3 (AP4_SAMPLE_FORMAT_EC_3) but lacks a well-formed dec3 child atom sends dec3 to NULL, and dec3->GetSubStreams()[0].acmod faults reading offset 12 into the zero page (@sigdevel's PoC on the mp42hls/5 sample). Even if dec3 is present, its substreams array can be empty — GetSubStreams()[0] then binds a reference to a null pointer via AP4_Array::operator[], and the deref faults at the same offset one line further down.

Guard the block right after the cast: reject the sample if dec3 is NULL or its substreams array has no entries, print an error matching the surrounding style (ERROR: E-AC-3 sample description is missing a usable dec3 atom, same shape as the sample_description == NULL message a few dozen lines below) and goto end to run the existing teardown. Valid E-AC-3 inputs always carry a dec3 atom with at least one substream, so the guard falls through and nothing observable changes on the happy path.

Reported-by: @sigdevel — thanks for the report and reproducer.

The E-AC-3 branch of the audio-stream setup in mp42hls does

    AP4_Dec3Atom* dec3 = AP4_DYNAMIC_CAST(AP4_Dec3Atom,
        sample_description->GetDetails().GetChild(AP4_ATOM_TYPE_DEC3));
    ...
    if (dec3->GetSubStreams()[0].acmod == 0) { ... }

with no check that the cast returned non-NULL, and no check that the
substreams array is non-empty. A crafted MP4 whose sample description
advertises EC-3 (AP4_SAMPLE_FORMAT_EC_3) but lacks a well-formed dec3
child atom sends dec3 to NULL, and dec3->GetSubStreams()[0].acmod
segfaults reading offset 12 into the zero page (issue axiomatic-systems#1043 / @sigdevel's
PoC on the 'mp42hls/5' sample). Even if the dec3 atom itself is present
the substreams array can be empty, in which case
GetSubStreams()[0] binds a reference to a null pointer and the deref
faults at the same offset one line further down.

Guard the block right after the cast: reject the sample if dec3 is
NULL or its substreams array has no entries, print an error matching
the surrounding style (e.g. the 'unable to parse video sample
description' path a few lines below), and 'goto end' to run the
existing teardown. Valid E-AC-3 inputs always carry a dec3 atom with at
least one substream, so nothing observable changes on the happy path.

Reported-by: Alexander Shvedov <sigdevel>
Closes axiomatic-systems#1043

Signed-off-by: Brandon Barrante <aetherai@aethersystems.net>
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.

[security] Mp42Hls/Mp42Hls.cpp:2080 SEGV in main

1 participant