pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/37992?usp=email ) Change subject: coverity CID#216829 ...................................................................... coverity CID#216829 Simplify the bounds checking of values received on the wire. Change-Id: I30beeac45ff2d8c08905986af9fabfda071ddc5b --- M src/rtp.c 1 file changed, 21 insertions(+), 13 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve diff --git a/src/rtp.c b/src/rtp.c index 77905f2..1da409c 100644 --- a/src/rtp.c +++ b/src/rtp.c @@ -115,14 +115,21 @@ int x_len; int csrc_len; - csrc_len = rtph->csrc_count << 2; - payload = msg->data + sizeof(struct rtp_hdr) + csrc_len; - payload_len = msg->len - sizeof(struct rtp_hdr) - csrc_len; - if (payload_len < 0) { - DEBUGPC(DLMUX, "received RTP frame too short (len = %d, " - "csrc count = %d)\n", msg->len, rtph->csrc_count); + if (msg->len < sizeof(struct rtp_hdr)) { + DEBUGPC(DLMUX, "received RTP frame too short for an RTP header (%d < %zu)\n", + msg->len, sizeof(*rtph)); return NULL; } + + csrc_len = sizeof(struct rtp_hdr) + (rtph->csrc_count << 2); + if (msg->len < csrc_len) { + DEBUGPC(DLMUX, "received RTP frame too short for its csrc (%u < %d, csrc_count = %d)\n", + msg->len, csrc_len, rtph->csrc_count); + return NULL; + } + payload = msg->data + csrc_len; + payload_len = msg->len - csrc_len; + if (rtph->extension) { if (payload_len < sizeof(struct rtp_x_hdr)) { DEBUGPC(DLMUX, "received RTP frame too short for " @@ -131,26 +138,27 @@ } rtpxh = (struct rtp_x_hdr *)payload; x_len = ntohs(rtpxh->length) * 4 + sizeof(struct rtp_x_hdr); - payload += x_len; - payload_len -= x_len; - if (payload_len < 0) { + if (x_len > payload_len) { DEBUGPC(DLMUX, "received RTP frame too short, " "extension header exceeds frame length\n"); return NULL; } + payload += x_len; + payload_len -= x_len; } if (rtph->padding) { + uint8_t padding_len; if (payload_len < 1) { DEBUGPC(DLMUX, "received RTP frame too short for " "padding length\n"); return NULL; } - payload_len -= payload[payload_len - 1]; - if (payload_len < 0) { - DEBUGPC(DLMUX, "received RTP frame with padding " - "greater than payload\n"); + padding_len = payload[payload_len - 1]; + if (payload_len < padding_len) { + DEBUGPC(DLMUX, "received RTP frame with padding greater than payload\n"); return NULL; } + payload_len -= padding_len; } *plen = payload_len; -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/37992?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I30beeac45ff2d8c08905986af9fabfda071ddc5b Gerrit-Change-Number: 37992 Gerrit-PatchSet: 8 Gerrit-Owner: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt;