Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1043.
The E-AC-3 branch of
mp42hls's audio-stream setup doeswith 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-formeddec3child atom sendsdec3to NULL, anddec3->GetSubStreams()[0].acmodfaults reading offset 12 into the zero page (@sigdevel's PoC on themp42hls/5sample). Even ifdec3is present, its substreams array can be empty —GetSubStreams()[0]then binds a reference to a null pointer viaAP4_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
dec3isNULLor 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 thesample_description == NULLmessage a few dozen lines below) andgoto endto 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.