Repository navigation
Conversation
eaud_pcap_dissect() derives the payload length and the offset of the next record from the record's incl_len without checking it against the bytes left in the file. load_pcap() then hands l5_data/l5_len to rtp_packet_parse_raw(), and with SRTP to eaud_crypto_decrypt(), which decrypts in place, so a record that claims more than the file holds is read (or written) past the end of the mapping. A 264-byte recording with incl_len 0x10000000 and the RTP P bit set makes extractaudio segfault reading the padding length in rtp_packet_parse_raw(). pcap_hdr_len is an int, so an incl_len close to UINT32_MAX on a non-IPv4 record also wraps the record length to 0 or a few bytes and the scan loop stalls or resyncs in the middle of a record. Return PCP_DSCT_TRNK for such records, the same way eaud_adhoc_dissect() already handles truncated adhoc files. Add a badargs sub-test that builds such a file from call1_g722_srtp and expects extractaudio to fail with exit status 1 rather than crash.
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.
Repro: cut
tests/extractaudio/call1_g722_srtp.a.rtpdown to its first record (264 bytes), set that record'sincl_lento 0x10000000 and the RTP P bit, and runextractaudio <name> out.wav. It segfaults inrtp_packet_parse_raw()reading the padding length 256 MiB past the mapping (ASan: SEGV at rtp.c:318, called fromload_pcap()).Cause:
eaud_pcap_dissect()takesincl_lenfrom the record header and never compares it with the bytes left in the file, soload_pcap()passes a payload length that runs off the mmap()ed file tortp_packet_parse_raw()and, when keys are given, to the in-placesrtp_unprotect()ineaud_crypto_decrypt().Fix: return
PCP_DSCT_TRNKwhenincl_lendoesn't fit, aseaud_adhoc_dissect()already does for adhoc recordings.pcap_hdr_lenis an int, so a non-IPv4 record withincl_len0xfffffff0 wraps the record length to 0 andextractaudio -Sspins on it forever on master. With the check it reports "broken or truncated PCAP file" and moves on. The same happens for a recording cut short in the middle of a record.The new badargs sub-test builds the bad file from call1_g722_srtp with dd and expects exit status 1. It segfaults on master. Valid recordings are unaffected: scan output and the decoded mono/stereo WAVs for every tests/extractaudio fixture are byte-identical before and after. I built and ran this on macOS, where only extractaudio builds (rtpproxy itself doesn't), so the rest of the suite wasn't run locally.