Skip to content

Commit eb361b4

Browse files
marcelhecko
authored andcommitted
Fix buffer overread vulnerabilities in RTP packet parsing
Add bounds checking to AmRtpPacket::parse() to prevent crashes and potential security issues from malformed RTP packets: - Add minimum packet size check (12 bytes) before accessing RTP header - Validate CSRC list fits within packet before reading extension header - Add overflow-safe extension header length validation - Check data exists before reading padding length byte These checks prevent out-of-bounds reads that could be triggered by maliciously crafted RTP packets received from the network.
1 parent 6a8e364 commit eb361b4

File tree

1 file changed

+32
-12
lines changed

1 file changed

+32
-12
lines changed

core/AmRtpPacket.cpp

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,13 @@ int AmRtpPacket::parse()
7474
assert(buffer);
7575
assert(b_size);
7676

77+
// Minimum size check: RTP header is 12 bytes
78+
if (b_size < sizeof(rtp_hdr_t)) {
79+
ERROR("RTP packet too small (%u bytes, need %zu)\n",
80+
b_size, sizeof(rtp_hdr_t));
81+
return -1;
82+
}
83+
7784
rtp_hdr_t* hdr = (rtp_hdr_t*)buffer;
7885
// ZRTP "Hello" packet has version == 0
7986
if ((hdr->version != RTP_VERSION) && (hdr->version != 0))
@@ -83,23 +90,31 @@ int AmRtpPacket::parse()
8390
return -1;
8491
}
8592

93+
// CC is 4 bits (0-15), so cc*4 is max 60 bytes - no overflow possible
8694
data_offset = sizeof(rtp_hdr_t) + (hdr->cc*4);
8795

96+
// Validate CSRC list fits in packet before accessing extension header
97+
if (data_offset > b_size) {
98+
ERROR("bad rtp packet: CSRC exceeds size (offset=%u;size=%u)\n",
99+
data_offset, b_size);
100+
return -1;
101+
}
102+
88103
if(hdr->x != 0) {
89-
//#ifndef WITH_ZRTP
90-
//if (AmConfig::IgnoreRTPXHdrs) {
91-
// skip the extension header
92-
//#endif
104+
// Need 4 bytes for extension header (2 bytes profile + 2 bytes length)
93105
if (b_size >= data_offset + 4) {
94-
data_offset +=
95-
ntohs(((rtp_xhdr_t*) (buffer + data_offset))->len)*4;
106+
// Extension length is count of 32-bit words after the 4-byte header
107+
unsigned int ext_words = ntohs(((rtp_xhdr_t*) (buffer + data_offset))->len);
108+
// Total extension size: 4-byte header + ext_words * 4 bytes
109+
// Check for overflow: ext_words max is 65535, *4 = 262140, fits in unsigned int
110+
unsigned int ext_size = 4 + (ext_words * 4);
111+
112+
if (ext_size > b_size - data_offset) {
113+
ERROR("bad rtp packet: extension exceeds size\n");
114+
return -1;
115+
}
116+
data_offset += ext_size;
96117
}
97-
// #ifndef WITH_ZRTP
98-
// } else {
99-
// DBG("RTP extension headers not supported.\n");
100-
// return -1;
101-
// }
102-
// #endif
103118
}
104119

105120
payload = hdr->pt;
@@ -117,6 +132,11 @@ int AmRtpPacket::parse()
117132
d_size = b_size - data_offset;
118133

119134
if(hdr->p){
135+
// Need at least 1 byte to read padding length
136+
if (d_size == 0) {
137+
ERROR("bad rtp packet (padding set but no data)\n");
138+
return -1;
139+
}
120140
if (buffer[b_size-1]>=d_size){
121141
ERROR("bad rtp packet (invalid padding size) !\n");
122142
return -1;

0 commit comments

Comments
 (0)