Fix integer underflow reading undersized rtp/sdp atoms - #100
Open
1820893135-pixel wants to merge 1 commit into
Open
Fix integer underflow reading undersized rtp/sdp atoms#1001820893135-pixel wants to merge 1 commit into
1820893135-pixel wants to merge 1 commit into
Conversation
MP4RtpAtom::ReadHntiType() and MP4SdpAtom::Read() compute the string length as GetEnd() - m_File.GetPosition() and pass it straight to MP4Malloc(). If the atom is declared so small that the read position has already passed its declared end, the subtraction underflows to a value close to 2^64 and MP4Malloc() attempts a ~2^64-byte allocation (ASan: allocation-size-too-big). Guard the position before computing the size and skip the read (the atom is malformed) instead. Fixes TechSmith#98.
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.
Fixes the integer underflow reported in #98.
Root cause
MP4RtpAtom::ReadHntiType()(and identicallyMP4SdpAtom::Read()) computes the SDP string length as:If the
rtp/sdpatom is declared so small (e.g. a size field that only covers the 8-byte header) that the read position has already passed the atom's declared end, theuint64_tsubtraction underflows to a value close to2^64.MP4Malloc(size + 1)then attempts a ~2^64-byte allocation and ASan aborts withallocation-size-too-big(requested0xfffffffffffffffd).Fix
Guard the position before computing the size; if it has reached/passed the atom's end, the atom is malformed — log a warning and skip the SDP read (the caller already
Skip()s to the end of the atom).Verified: the reproducer from #98 aborts with
allocation-size-too-bigon the unfixed build and parses cleanly (exit 0, no sanitizer report) with this change under-fsanitize=address,undefined -fno-sanitize-recover=all.Fixes #98.