Repository navigation
Conversation
…SetDataSize AP4_SgpdAtom's constructor tracks the atom's remaining bytes with an unsigned AP4_Size counter and pattern-matches every read as 'ReadUI32(field); bytes_available -= 4;' - never checking that the four bytes were actually available before decrementing. On a truncated sgpd atom (size close to AP4_FULL_ATOM_HEADER_SIZE), the first '-= 4' wraps bytes_available to 0xFFFFFFFC. The entry loop below then computes description_length as either bytes_available (version 0) or as a value it compares against bytes_available (version 1). Either way the comparison 'description_length <= bytes_available' passes any value up to 0xFFFFFFFC, and AP4_DataBuffer::SetDataSize is handed the same, which is then forwarded to operator new[] as a ~4 GiB allocation request - AddressSanitizer aborts with 'out of memory: allocator is trying to allocate 0xfffffffc bytes'. See @sigdevel's PoC linked from gh#1042 (also referenced by GHSA-v7gf-c3mw-55gx). Guard each read-and-decrement with 'if (bytes_available < 4) return;'. Well-formed inputs have plenty of room so nothing changes on the happy path; malformed atoms exit the constructor early with the previously-read fields (m_GroupingType and optionally m_DefaultLength) at their default values and an empty m_Entries list. Reported-by: Alexander Shvedov <sigdevel> Closes axiomatic-systems#1042 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
1 participant
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 #1042.
AP4_SgpdAtom's constructor tracks the atom's remaining bytes in an unsignedAP4_Sizecounter and pattern-matches every read asReadUI32(field); bytes_available -= 4;— no check that four bytes were actually left before the decrement.On a truncated sgpd (crafted PoC has size close to
AP4_FULL_ATOM_HEADER_SIZE), the first-=4wrapsbytes_availableto0xFFFFFFFC. The entry loop below computesdescription_lengthas eitherbytes_available(version 0) or compares a crafted value against it (version 1); either way thedescription_length <= bytes_availablecheck passes anything up to0xFFFFFFFC, andAP4_DataBuffer::SetDataSizeforwards that same value tooperator new[]as a ~4 GiB allocation. ASan aborts:ERROR: AddressSanitizer: out of memory: allocator is trying to allocate 0xfffffffc bytes
#0 operator new[]
#1 AP4_DataBuffer::ReallocateBuffer Ap4DataBuffer.cpp:210
#2 AP4_DataBuffer::SetDataSize Ap4DataBuffer.cpp:151
#3 AP4_SgpdAtom::AP4_SgpdAtom Ap4SgpdAtom.cpp:108
Guard each read-and-decrement in the constructor prologue with
if (bytes_available < 4) return;. Well-formed inputs have plenty of room so nothing observable changes; a malformed atom exits the constructor early withm_GroupingType/m_DefaultLengthstill at their default-initialised values and an emptym_Entries— the same shape the code already produces when one of the reads fails silently.Local build (Ubuntu 24.04, gcc 13.3.0, cmake
-DCMAKE_CXX_FLAGS="-fsanitize=address,undefined -g -O0") reproduces the ASan OOM on masterb8c50a0and comes back clean after this patch:$ ./mp4info ~/bento4-poc/poc4 # master b8c50a0
==*==ERROR: AddressSanitizer: out of memory: allocator is trying to allocate 0xfffffffc bytes
exit=1
$ ./mp4info ~/bento4-poc/poc4 # fix 8743f43
Movie:
duration: 1181314149 (movie timescale units)
...
Found 2 Tracks
exit=0
Reported-by: @sigdevel — thanks for the report and reproducer, also referenced by GHSA-v7gf-c3mw-55gx.