From 5887f6a6ec3b144864d8039232515c193d481c28 Mon Sep 17 00:00:00 2001 From: Chema Gonzalez Date: Fri, 16 Oct 2026 02:47:24 -0700 Subject: [PATCH 1/2] Parse RTP header extensions and H.264 bounds safely Account for CSRC lists, header extensions, and padding before media depacketization. Reject invalid RTP versions and truncated extension or padding fields before exposing payload bytes. Bound generic payloads and H.264 single-NAL, STAP-A, and FU-A inputs. Reject empty NAL units, oversized fragments, continuation fragments without a start, and trailing incomplete STAP-A data. Add focused regressions for valid extension and padding layouts, malformed lengths, H.264 packetization bounds, and generic payload limits. Co-authored-by: Claude Co-authored-by: Codex --- src/rtp.c | 61 ++++++++++++++++++++++++++--------- tests/test_chrome_rtp.c | 70 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 15 deletions(-) create mode 100644 tests/test_chrome_rtp.c diff --git a/src/rtp.c b/src/rtp.c index 21d29f71..e610a78b 100644 --- a/src/rtp.c +++ b/src/rtp.c @@ -35,7 +35,7 @@ typedef struct FuHeader { #define FU_HEADER_SIZE 1 int rtp_packet_validate(uint8_t* packet, size_t size) { - if (size < 12) + if (size < 12 || (packet[0] >> 6) != 2) return 0; RtpHeader* rtp_header = (RtpHeader*)packet; @@ -121,6 +121,7 @@ static int rtp_encoder_encode_h264_fu_a(RtpEncoder* rtp_encoder, uint8_t* buf, s } static uint8_t* h264_find_nalu(uint8_t* buf_start, uint8_t* buf_end) { + if ((size_t)(buf_end - buf_start) < 3) return buf_end; uint8_t* p = buf_start + 2; while (p < buf_end) { @@ -144,8 +145,9 @@ static int rtp_encoder_encode_h264(RtpEncoder* rtp_encoder, uint8_t* buf, size_t if (pend != buf_end) nalu_size--; - while (pstart[nalu_size - 1] == 0x00) + while (nalu_size && pstart[nalu_size - 1] == 0x00) nalu_size--; + if (!nalu_size) continue; if (nalu_size <= RTP_PAYLOAD_SIZE) { rtp_encoder_encode_h264_single(rtp_encoder, pstart, nalu_size); @@ -159,6 +161,7 @@ static int rtp_encoder_encode_h264(RtpEncoder* rtp_encoder, uint8_t* buf, size_t } static int rtp_encoder_encode_generic(RtpEncoder* rtp_encoder, uint8_t* buf, size_t size) { + if (size > RTP_PAYLOAD_SIZE) return -1; RtpHeader* rtp_header = (RtpHeader*)rtp_encoder->buf; rtp_header_init(rtp_header, rtp_encoder->type); rtp_header->seq_number = htons(rtp_encoder->seq_number); @@ -210,6 +213,7 @@ void rtp_encoder_init(RtpEncoder* rtp_encoder, MediaCodec codec, RtpOnPacket on_ } int rtp_encoder_encode(RtpEncoder* rtp_encoder, const uint8_t* buf, size_t size) { + if (!buf || !size || !rtp_encoder->encode_func) return -1; return rtp_encoder->encode_func(rtp_encoder, (uint8_t*)buf, size); } @@ -220,11 +224,12 @@ static int rtp_decode_h264_stap_a(RtpDecoder* rtp_decoder, uint8_t* nalu_buf, int* nalu_offset) { *nalu_offset = 0; - while (*nalu_offset + 2 < size) { - uint16_t nalu_length = buf[*nalu_offset] << 8 | buf[*nalu_offset + 1]; + while ((size_t)*nalu_offset + 2 <= size) { + size_t nalu_length = (size_t)buf[*nalu_offset] << 8 | buf[*nalu_offset + 1]; *nalu_offset += 2; - if (*nalu_offset + nalu_length > size) { + if (!nalu_length || nalu_length > CONFIG_MAX_NALU_SIZE - NALU_START_CODE_SIZE || + (size_t)*nalu_offset + nalu_length > size) { LOGE("Invalid STAP-A packet: NALU length exceeds packet size"); return -1; } @@ -238,6 +243,10 @@ static int rtp_decode_h264_stap_a(RtpDecoder* rtp_decoder, *nalu_offset += nalu_length; } + if ((size_t)*nalu_offset != size) { + *nalu_offset = 0; + return -1; + } return 0; } @@ -246,6 +255,7 @@ static int rtp_decode_h264_single(RtpDecoder* rtp_decoder, size_t size, uint8_t* nalu_buf, int* nalu_offset) { + if (!size || size > CONFIG_MAX_NALU_SIZE - NALU_START_CODE_SIZE) return -1; memcpy(nalu_buf, &nalu_start_4bytecode, NALU_START_CODE_SIZE); *nalu_offset = NALU_START_CODE_SIZE; memcpy(nalu_buf + *nalu_offset, buf, size); @@ -262,6 +272,10 @@ static int rtp_decode_h264_fu_a(RtpDecoder* rtp_decoder, size_t size, uint8_t* nalu_buf, int* nalu_offset) { + if (size < NALU_HEADER_SIZE + FU_HEADER_SIZE) { + *nalu_offset = 0; + return -1; + } NaluHeader* fu_indicator = (NaluHeader*)buf; FuHeader* fu_header = (FuHeader*)(buf + NALU_HEADER_SIZE); uint8_t reconstructed_nalu_type = (fu_indicator->f << 7) | @@ -269,6 +283,12 @@ static int rtp_decode_h264_fu_a(RtpDecoder* rtp_decoder, fu_header->type; buf += NALU_HEADER_SIZE + FU_HEADER_SIZE; size -= NALU_HEADER_SIZE + FU_HEADER_SIZE; + size_t required = fu_header->s ? NALU_START_CODE_SIZE + 1 : (size_t)*nalu_offset; + if ((!fu_header->s && !*nalu_offset) || required > CONFIG_MAX_NALU_SIZE || + size > CONFIG_MAX_NALU_SIZE - required) { + *nalu_offset = 0; + return -1; + } if (fu_header->s) { memcpy(nalu_buf, &nalu_start_4bytecode, NALU_START_CODE_SIZE); *nalu_offset = NALU_START_CODE_SIZE; @@ -293,26 +313,23 @@ static int rtp_decode_h264_fu_a(RtpDecoder* rtp_decoder, static int rtp_decode_h264(RtpDecoder* rtp_decoder, uint8_t* buf, size_t size) { static uint8_t nalu_buf[CONFIG_MAX_NALU_SIZE]; static int offset = 0; - RtpPacket* rtp_packet = (RtpPacket*)buf; - uint8_t nalu_type = *rtp_packet->payload & 0x1f; - int payload_size = size - sizeof(RtpHeader); + uint8_t nalu_type = *buf & 0x1f; switch (nalu_type) { case STAP_A: - return rtp_decode_h264_stap_a(rtp_decoder, rtp_packet->payload + 1, payload_size - 1, nalu_buf, &offset); + return rtp_decode_h264_stap_a(rtp_decoder, buf + 1, size - 1, nalu_buf, &offset); case FU_A: - return rtp_decode_h264_fu_a(rtp_decoder, rtp_packet->payload, payload_size, nalu_buf, &offset); + return rtp_decode_h264_fu_a(rtp_decoder, buf, size, nalu_buf, &offset); default: - return rtp_decode_h264_single(rtp_decoder, rtp_packet->payload, payload_size, nalu_buf, &offset); + return rtp_decode_h264_single(rtp_decoder, buf, size, nalu_buf, &offset); break; } return 0; } static int rtp_decode_generic(RtpDecoder* rtp_decoder, uint8_t* buf, size_t size) { - RtpPacket* rtp_packet = (RtpPacket*)buf; if (rtp_decoder->on_packet != NULL) - rtp_decoder->on_packet(rtp_packet->payload, size - sizeof(RtpHeader), rtp_decoder->user_data); + rtp_decoder->on_packet(buf, size, rtp_decoder->user_data); // even if there is no callback set, assume everything is ok for caller and do not return an error return (int)size; } @@ -335,7 +352,21 @@ void rtp_decoder_init(RtpDecoder* rtp_decoder, MediaCodec codec, RtpOnPacket on_ } int rtp_decoder_decode(RtpDecoder* rtp_decoder, const uint8_t* buf, size_t size) { - if (rtp_decoder->decode_func == NULL) + if (!buf || !rtp_decoder->decode_func || size < sizeof(RtpHeader) || (buf[0] >> 6) != 2) return -1; - return rtp_decoder->decode_func(rtp_decoder, (uint8_t*)buf, size); + size_t offset = sizeof(RtpHeader) + 4 * (buf[0] & 0x0f); + if (offset > size) return -1; + if (buf[0] & 0x10) { + if (size - offset < 4) return -1; + size_t extension = 4 + 4 * ((size_t)buf[offset + 2] << 8 | buf[offset + 3]); + if (extension > size - offset) return -1; + offset += extension; + } + if (buf[0] & 0x20) { + size_t padding = buf[size - 1]; + if (!padding || padding > size - offset) return -1; + size -= padding; + } + if (offset >= size) return -1; + return rtp_decoder->decode_func(rtp_decoder, (uint8_t*)buf + offset, size - offset); } diff --git a/tests/test_chrome_rtp.c b/tests/test_chrome_rtp.c new file mode 100644 index 00000000..687025d0 --- /dev/null +++ b/tests/test_chrome_rtp.c @@ -0,0 +1,70 @@ +#include +#include +#include +#include "rtp.h" + +static uint8_t received[4096]; +static size_t received_size; +static int packets; +static void on_packet(uint8_t* data, size_t size, void* unused) { + (void)unused; + assert(size <= sizeof(received)); + memcpy(received, data, size); + received_size = size; + packets++; +} + +int main(void) { + RtpDecoder decoder = {0}; + RtpEncoder encoder = {0}; + uint8_t packet[64] = {0x91, 0}; /* V2, X, one CSRC */ + packet[16] = 0xbe; packet[17] = 0xde; + packet[19] = 1; /* One extension word */ + packet[24] = 0xff; packet[25] = 0x7f; + rtp_decoder_init(&decoder, CODEC_PCMU, on_packet, NULL); + assert(rtp_decoder_decode(&decoder, packet, 26) >= 0); + assert(received_size == 2 && received[0] == 0xff && received[1] == 0x7f); + packet[0] |= 0x20; + packet[26] = 0; packet[27] = 2; + assert(rtp_decoder_decode(&decoder, packet, 28) >= 0 && received_size == 2); + packet[27] = 0; + assert(rtp_decoder_decode(&decoder, packet, 28) < 0); + packet[0] &= ~0x20; + packet[19] = 255; + assert(rtp_decoder_decode(&decoder, packet, 26) < 0); + packet[19] = 1; + assert(rtp_decoder_decode(&decoder, packet, 24) < 0); + assert(rtp_decoder_decode(&decoder, packet, 17) < 0); + packet[0] = 0x8f; + assert(rtp_decoder_decode(&decoder, packet, 26) < 0); + packet[0] = 0x91; + rtp_decoder_init(&decoder, CODEC_H264, on_packet, NULL); + packet[24] = 0x65; packet[25] = 0xab; + assert(rtp_decoder_decode(&decoder, packet, 26) == 0); + assert(received_size == 6 && received[3] == 1 && received[4] == 0x65 && received[5] == 0xab); + uint8_t stap[17] = {0x80, PT_H264}; + stap[12] = 24; + stap[14] = 1; + stap[15] = 0x65; + assert(rtp_decoder_decode(&decoder, stap, 16) == 0); + stap[16] = 0xff; + assert(rtp_decoder_decode(&decoder, stap, sizeof(stap)) < 0); + packet[24] = 28; + assert(rtp_decoder_decode(&decoder, packet, 25) < 0); + packet[25] = 5; /* continuation without start */ + assert(rtp_decoder_decode(&decoder, packet, 26) < 0); + rtp_encoder_init(&encoder, CODEC_PCMU, on_packet, NULL); + uint8_t large[CONFIG_MTU] = {0}; + assert(rtp_encoder_encode(&encoder, large, sizeof(large)) < 0); + rtp_encoder_init(&encoder, CODEC_H264, on_packet, NULL); + encoder.type = 125; + const uint8_t nal[] = {0, 0, 0, 1, 0x65, 0xab}; + assert(rtp_encoder_encode(&encoder, nal, sizeof(nal)) == 0); + assert((received[1] & 0x7f) == 125); + assert(rtp_encoder_encode(&encoder, NULL, 0) < 0); + const uint8_t empty_nal[] = {0, 0, 1, 0, 0, 0}; + int before = packets; + assert(rtp_encoder_encode(&encoder, empty_nal, sizeof(empty_nal)) == 0 && before == packets); + puts("Chrome RTP extension/bounds regression: PASS"); + return 0; +} From ea709f6b294e15c6e8f16d5f719c45067ed7e16b Mon Sep 17 00:00:00 2001 From: Chema Gonzalez Date: Fri, 16 Oct 2026 22:05:16 -0700 Subject: [PATCH 2/2] Retain full-size SRTP RTP packets libSRTP can append up to SRTP_MAX_TRAILER_LEN bytes while protecting an RTP packet. The encoder and peer receive path reserved a hard-coded 128 bytes, less than the current libSRTP maximum of 144 bytes. Size both packet buffers from the library constant so a CONFIG_MTU packet can be protected and received without truncating or exceeding storage. Extend the RTP regression with an MTU-spanning H.264 FU-A round trip and assert the shared protected-packet capacity. Co-authored-by: Claude Co-authored-by: Codex --- src/peer_connection.c | 8 +++++++- src/rtp.h | 6 +++++- tests/test_chrome_rtp.c | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/peer_connection.c b/src/peer_connection.c index 91136de5..cdc76ea6 100644 --- a/src/peer_connection.c +++ b/src/peer_connection.c @@ -36,7 +36,8 @@ struct PeerConnection { void (*on_receiver_packet_loss)(float fraction_loss, uint32_t total_loss, void* user_data); uint8_t temp_buf[CONFIG_MTU]; - uint8_t agent_buf[CONFIG_MTU]; + /* SRTP authentication/tag overhead is added after RTP packetization. */ + uint8_t agent_buf[RTP_PACKET_BUFFER_SIZE]; int agent_ret; RtpEncoder artp_encoder; @@ -48,6 +49,11 @@ struct PeerConnection { uint32_t remote_vssrc; }; +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L +_Static_assert(sizeof(((PeerConnection*)0)->agent_buf) >= RTP_PACKET_BUFFER_SIZE, + "agent buffer must hold a protected RTP packet"); +#endif + static void peer_connection_outgoing_rtp_packet(uint8_t* data, size_t size, void* user_data) { PeerConnection* pc = (PeerConnection*)user_data; int packet_len = (int)size; diff --git a/src/rtp.h b/src/rtp.h index db6a11d1..b269b467 100644 --- a/src/rtp.h +++ b/src/rtp.h @@ -3,6 +3,8 @@ #include +#include + #ifdef __BYTE_ORDER #define __BIG_ENDIAN 4321 #define __LITTLE_ENDIAN 1234 @@ -74,6 +76,8 @@ typedef struct RtpEncoder RtpEncoder; typedef struct RtpDecoder RtpDecoder; typedef void (*RtpOnPacket)(uint8_t* packet, size_t bytes, void* user_data); +#define RTP_PACKET_BUFFER_SIZE (CONFIG_MTU + SRTP_MAX_TRAILER_LEN) + struct RtpDecoder { RtpPayloadType type; RtpOnPacket on_packet; @@ -90,7 +94,7 @@ struct RtpEncoder { uint32_t ssrc; uint32_t timestamp; uint32_t timestamp_increment; - uint8_t buf[CONFIG_MTU + 128]; + uint8_t buf[RTP_PACKET_BUFFER_SIZE]; }; int rtp_packet_validate(uint8_t* packet, size_t size); diff --git a/tests/test_chrome_rtp.c b/tests/test_chrome_rtp.c index 687025d0..e90494d7 100644 --- a/tests/test_chrome_rtp.c +++ b/tests/test_chrome_rtp.c @@ -6,6 +6,7 @@ static uint8_t received[4096]; static size_t received_size; static int packets; +static RtpDecoder roundtrip_decoder; static void on_packet(uint8_t* data, size_t size, void* unused) { (void)unused; assert(size <= sizeof(received)); @@ -13,10 +14,15 @@ static void on_packet(uint8_t* data, size_t size, void* unused) { received_size = size; packets++; } +static void decode_packet(uint8_t* data, size_t size, void* unused) { + (void)unused; + assert(rtp_decoder_decode(&roundtrip_decoder, data, size) == 0); +} int main(void) { RtpDecoder decoder = {0}; RtpEncoder encoder = {0}; + assert(sizeof(encoder.buf) == RTP_PACKET_BUFFER_SIZE); uint8_t packet[64] = {0x91, 0}; /* V2, X, one CSRC */ packet[16] = 0xbe; packet[17] = 0xde; packet[19] = 1; /* One extension word */ @@ -61,6 +67,17 @@ int main(void) { const uint8_t nal[] = {0, 0, 0, 1, 0x65, 0xab}; assert(rtp_encoder_encode(&encoder, nal, sizeof(nal)) == 0); assert((received[1] & 0x7f) == 125); + uint8_t fragmented[1469]; + memset(fragmented, 0xab, sizeof(fragmented)); + fragmented[0] = fragmented[1] = fragmented[2] = 0; + fragmented[3] = 1; + fragmented[4] = 0x65; + received_size = 0; + rtp_decoder_init(&roundtrip_decoder, CODEC_H264, on_packet, NULL); + rtp_encoder_init(&encoder, CODEC_H264, decode_packet, NULL); + assert(rtp_encoder_encode(&encoder, fragmented, sizeof(fragmented)) == 0); + assert(received_size == sizeof(fragmented)); + assert(memcmp(received, fragmented, sizeof(fragmented)) == 0); assert(rtp_encoder_encode(&encoder, NULL, 0) < 0); const uint8_t empty_nal[] = {0, 0, 1, 0, 0, 0}; int before = packets;