ben.carrasco/freeswitch
1
0
Fork 0
mirror of https://github.com/signalwire/freeswitch.git synced 2026-08-19 09:40:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge commit from fork

Browse Source 
The RTCP NACK feedback handler used the packet's header length field as the
loop bound instead of the byte count the caller already validated. An
oversized length ran the loop past the fixed-size RTCP buffer, an
out-of-bounds access.

Clamp the entry count to the feedback words that fit in the received packet.

Add an end-to-end test feeding an oversized-length NACK through the RTCP
reader.
This commit is contained in:
Dmitry Verenitsin
2026-08-08 18:29:25 +03:00
committed by GitHub
parent d557ca6d29
commit e730a4e541
2 changed files with 125 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/switch_rtp.c
+29 -3
View File
@@ -7108,6 +7108,32 @@ static switch_status_t read_rtp_packet(switch_rtp_t *rtp_session, switch_size_t
return status;
}
/* Number of NACK FCI entries (each a 32-bit word) carried in extp, capped to what fits past the
RTPFB header in block_len bytes. block_len is the length of this RTCP block in bytes; the
header's own length field is honored only up to that cap. */
static int rtcp_nack_fci_count(const rtcp_ext_msg_t *extp, switch_size_t block_len)
{
int claimed, fits;
/* No room for any FCI entry past the fixed RTPFB header (also keeps the unsigned
subtraction below from wrapping on a short block). */
if (block_len <= sizeof(switch_rtcp_ext_hdr_t)) {
return 0;
}
/* RTCP length is the packet size in 32-bit words minus one; the RTPFB header is three words
(common header plus two SSRCs), leaving (length - 2) words of NACK FCI entries. */
claimed = ntohs(extp->header.length) - 2;
if (claimed <= 0) {
return 0;
}
/* Entries that fit in the validated block length. */
fits = (int) ((block_len - sizeof(switch_rtcp_ext_hdr_t)) / sizeof(uint32_t));
return claimed < fits ? claimed : fits;
}
static void handle_nack(switch_rtp_t *rtp_session, uint32_t nack)
{
switch_size_t bytes = 0;
@@ -7224,13 +7250,13 @@ static switch_status_t process_rtcp_report(switch_rtp_t *rtp_session, rtcp_msg_t
if (msg->header.type == _RTCP_PT_RTPFB && extp->header.fmt == _RTCP_RTPFB_NACK) {
uint32_t *nack = (uint32_t *) extp->body;
int i;
int i, nack_count = rtcp_nack_fci_count(extp, bytes);
switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_DEBUG2, "%s Got NACK count %d\n",
switch_core_session_get_name(rtp_session->session), ntohs(extp->header.length) - 2);
switch_core_session_get_name(rtp_session->session), nack_count);
for (i = 0; i < ntohs(extp->header.length) - 2; i++) {
for (i = 0; i < nack_count; i++) {
handle_nack(rtp_session, nack[i]);
}
tests/unit/switch_rtp_pcap.c
+96 -1
View File
@@ -246,7 +246,12 @@ static switch_status_t rtp_test_end_call(switch_core_session_t **psession)
switch_channel_hangup(channel, SWITCH_CAUSE_NORMAL_CLEARING);
switch_media_handle_destroy(session);
switch_core_session_rwunlock(session);
/* switch_media_handle_destroy() already destroyed the session's RTP session; clear both
pointers so callers and later tests never act on the freed objects. */
*psession = NULL;
rtp_session = NULL;
return SWITCH_STATUS_SUCCESS;
}
@@ -450,6 +455,96 @@ FST_TEARDOWN_END()
FST_TEST_END()
#endif
FST_TEST_BEGIN(test_rtcp_nack_oversized_length)
{
switch_core_session_t *session = NULL;
switch_status_t status;
switch_socket_t *sock_rtp = NULL;
switch_sockaddr_t *sock_addr = NULL;
const char *str_err = NULL;
char rpacket[SWITCH_RECOMMENDED_BUFFER_SIZE];
switch_payload_t pt = { 0 };
switch_frame_flag_t frameflags = { 0 };
uint32_t rcvd_datalen;
switch_size_t send_len;
int i;
/* RTPFB NACK feedback: V=2/P=0/FMT=1, PT=205 (byte 0xCD -> header.pt 77), header.length
claims 0xFFFF words but only two FCI words are present. The reader must process two
entries (clamped to the datagram), not the 0xFFFF - 2 implied by the length field. */
unsigned char nack[20] = {
0x81, 0xcd, 0xff, 0xff, /* V/P/FMT=1, PT=205, length=0xFFFF */
0xde, 0xad, 0xbe, 0xef, /* sender SSRC */
0xca, 0xfe, 0xba, 0xbe, /* media SSRC */
0x00, 0x01, 0x00, 0x00, /* FCI word 0: PID=1, BLP=0 */
0x00, 0x02, 0x00, 0x00 /* FCI word 1: PID=2, BLP=0 */
};
status = rtp_test_start_call(&session);
if (status != SWITCH_STATUS_SUCCESS || !session) {
fst_fail("failed to start PCMU test call");
goto nack_oob_cleanup;
}
switch_core_media_set_rtp_flag(session, SWITCH_MEDIA_TYPE_AUDIO, SWITCH_RTP_FLAG_ENABLE_RTCP);
rtp_session = switch_core_media_get_rtp_session(session, SWITCH_MEDIA_TYPE_AUDIO);
if (!rtp_session) {
fst_fail("no RTP session");
goto nack_oob_cleanup;
}
switch_rtp_clear_flag(rtp_session, SWITCH_RTP_FLAG_PAUSE);
/* This test feeds only RTCP; drop the media timeout so the no-RTP reads don't return a
media-timeout error that the post-condition below would misread as a reader failure. */
switch_rtp_set_media_timeout(rtp_session, 0);
/* Accept the feedback packet from the test socket and route it through the video
NACK branch of the RTCP reader. */
switch_rtp_set_flag(rtp_session, SWITCH_RTP_FLAG_AUTOADJ);
switch_rtp_set_flag(rtp_session, SWITCH_RTP_FLAG_VIDEO);
switch_rtp_set_flag(rtp_session, SWITCH_RTP_FLAG_NACK);
if (switch_socket_create(&sock_rtp, AF_INET, SOCK_DGRAM, 0, switch_core_session_get_pool(session)) != SWITCH_STATUS_SUCCESS) {
fst_fail("failed to create test socket");
goto nack_oob_cleanup;
}
switch_sockaddr_new(&sock_addr, rx_host, audio_rx_port, switch_core_session_get_pool(session));
if (!sock_addr) {
fst_fail("failed to create sockaddr");
goto nack_oob_cleanup;
}
switch_rtp_set_remote_address(rtp_session, tx_host, switch_sockaddr_get_port(sock_addr), 0, SWITCH_FALSE, &str_err);
switch_rtp_reset(rtp_session);
/* rtcp-mux: the feedback arrives on the RTP socket. Send and read a few rounds so the
source address is adopted and the packet reaches the RTCP NACK parser. With the FCI
count clamped the parser stays in bounds; without it the read trips an out-of-bounds
access (ASan/Valgrind abort, or heap corruption). */
for (i = 0; i < 4; i++) {
send_len = sizeof(nack);
if (switch_socket_sendto(sock_rtp, sock_addr, MSG_CONFIRM, (const char *)nack, &send_len) != SWITCH_STATUS_SUCCESS) {
fst_fail("failed to send NACK packet");
goto nack_oob_cleanup;
}
rcvd_datalen = sizeof(rpacket);
status = switch_rtp_read(rtp_session, (void *)rpacket, &rcvd_datalen, &pt, &frameflags, io_flags);
}
/* In-band, synchronous post-condition: the read we just issued on this thread returned a
non-error status, i.e. the reader tolerated the malformed feedback rather than breaking.
The out-of-bounds access itself is caught by ASan/Valgrind on the read above. */
fst_xcheck(status != SWITCH_STATUS_GENERR && status != SWITCH_STATUS_FALSE,
"RTP reader tolerates an oversized RTCP NACK and keeps reading");
nack_oob_cleanup:
if (sock_rtp) switch_socket_close(sock_rtp);
/* rtp_test_end_call() -> switch_media_handle_destroy() destroys the RTP session. */
if (session) rtp_test_end_call(&session);
}
FST_TEST_END()
FST_TEST_BEGIN(test_rtp_media_timeout)
{
switch_core_session_t *session = NULL;