Skip to content

AmRtpStream::recvDtmfPacket: reject telephone-event packets with unexpected size#552

Open
hecko wants to merge 1 commit into
masterfrom
hecko/amrtpstream-recvdtmf-size-guard
Open

AmRtpStream::recvDtmfPacket: reject telephone-event packets with unexpected size#552
hecko wants to merge 1 commit into
masterfrom
hecko/amrtpstream-recvdtmf-size-guard

Conversation

@hecko

@hecko hecko commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Bug

AmRtpStream::recvDtmfPacket (core/AmRtpStream.cpp) casts the RTP payload straight to dtmf_payload_t* and reads all of its fields without checking that the packet actually carries a full payload:

void AmRtpStream::recvDtmfPacket(AmRtpPacket* p) {
  if (p->payload == getLocalTelephoneEventPT()) {
    dtmf_payload_t* dpl = (dtmf_payload_t*)p->getData();
    DBG(... dpl->event, dpl->e, dpl->r, dpl->volume, ntohs(dpl->duration) ...);
    if (session)
      session->postDtmfEvent(new AmRtpDtmfEvent(dpl, getLocalTelephoneEventRate(), p->timestamp));
  }
}

dtmf_payload_t is 4 bytes (event, flags/volume byte, 2-byte duration). A telephone-event RTP packet carrying only 1–3 payload bytes still passes the non-empty-payload filter in receive() (if(!rp->getDataSize())), so the cast reads dpl->duration (and possibly volume) past the received data. The result is a bogus AmRtpDtmfEvent with 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):

if (p->getDataSize() != sizeof(dtmf_payload_t))
  return;

Validation

  • Confirmed the unchecked cast is present in current master and that the only size filter before it (receive()) merely rejects a zero-length payload, not short ones.
  • The guard only adds an early return for malformed packets; well-formed 4-byte telephone-event payloads are unaffected.

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

…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.
Copilot AI review requested due to automatic review settings July 10, 2026 03:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 thread core/AmRtpStream.cpp
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants