AmRtpStream::recvDtmfPacket: reject telephone-event packets with unexpected size#552
Open
hecko wants to merge 1 commit into
Open
AmRtpStream::recvDtmfPacket: reject telephone-event packets with unexpected size#552hecko wants to merge 1 commit into
hecko wants to merge 1 commit into
Conversation
…pected size recvDtmfPacket casts the RTP payload to dtmf_payload_t* and reads event/e/r/volume/duration without checking that the packet actually carries a full 4-byte telephone-event payload. A truncated (1-3 byte) telephone-event packet passes the non-empty-payload filter in receive() and makes the cast read past the received data, producing bogus DTMF events from stale buffer bytes. Reject telephone-event packets whose data size is not exactly sizeof(dtmf_payload_t). Backport of yeti-switch/sems a15aa819.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens DTMF (RFC 2833/telephone-event) handling in AmRtpStream::recvDtmfPacket by validating the RTP payload length before casting it to dtmf_payload_t, preventing reads past the received payload and avoiding posting malformed DTMF events.
Changes:
- Add a guard to reject telephone-event RTP packets whose payload size is not exactly
sizeof(dtmf_payload_t). - Document why the size check is required (prevents bogus event/volume/duration).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+858
to
+860
| // a telephone-event payload is exactly sizeof(dtmf_payload_t) bytes; a | ||
| // truncated packet would make the cast below read past the received data | ||
| // and yield bogus event/volume/duration values. |
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.
Bug
AmRtpStream::recvDtmfPacket(core/AmRtpStream.cpp) casts the RTP payload straight todtmf_payload_t*and reads all of its fields without checking that the packet actually carries a full payload:dtmf_payload_tis 4 bytes (event, flags/volume byte, 2-byteduration). A telephone-event RTP packet carrying only 1–3 payload bytes still passes the non-empty-payload filter inreceive()(if(!rp->getDataSize())), so the cast readsdpl->duration(and possiblyvolume) past the received data. The result is a bogusAmRtpDtmfEventwith garbage volume/duration posted to the session. The payload type and length are attacker-controlled, so a remote peer can inject spurious DTMF signaling.Fix
Reject telephone-event packets whose data size is not exactly
sizeof(dtmf_payload_t):Validation
masterand that the only size filter before it (receive()) merely rejects a zero-length payload, not short ones.Acknowledgement
Backport of the fix from the yeti-switch/sems fork, commit
a15aa819152f10f24fcfb35f975ef025ba5fa3f7("AmRtpStream::recvDtmfPacket: ignore telephone-event packets with unexpected size"). Thanks to the yeti-switch maintainers.Generated by Claude Code