neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-mgw/+/15242
Change subject: fix crashes: don't assert on incoming RTP packet size
......................................................................
fix crashes: don't assert on incoming RTP packet size
Remove various OSMO_ASSERT() on size of incoming packets. Doing an assert on
incoming data is a DoS attack vector, absolute no-go. Instead, return -EINVAL
and keep running.
Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 31 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/42/15242/1
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index bb29d2b..d5f197d 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -475,7 +475,12 @@
uint8_t pt_in;
int pt_out;
- OSMO_ASSERT(len >= sizeof(struct rtp_hdr));
+ if (msg->len < sizeof(struct rtp_hdr)) {
+ LOG_CONN_RTP(conn_src, LOGL_ERROR, "RTP packet too short (%u <
%zu)\n",
+ msg->len, sizeof(struct rtp_hdr));
+ return -EINVAL;
+ }
+
rtp_hdr = (struct rtp_hdr *)data;
pt_in = rtp_hdr->payload_type;
@@ -641,7 +646,7 @@
* the receiving end expects GSM-HR data to be formated after RFC 5993, this
* function is used to convert between RFC 5993 and TS 101318, which we
normally
* use. */
-static void rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int
*len)
+static int rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int *len)
{
/* NOTE: *data has an overall length of RTP_BUF_SIZE, so there is
* plenty of space available to store the slightly larger, converted
@@ -649,7 +654,12 @@
struct rtp_hdr *rtp_hdr;
- OSMO_ASSERT(*len >= sizeof(struct rtp_hdr));
+ if (*len < sizeof(struct rtp_hdr)) {
+ LOGPENDP(endp, DRTP, LOGL_ERROR, "AMR RTP packet too short (%d
< %zu)\n",
+ *len, sizeof(struct rtp_hdr));
+ return -EINVAL;
+ }
+
rtp_hdr = (struct rtp_hdr *)data;
if (*len == GSM_HR_BYTES + sizeof(struct rtp_hdr)) {
@@ -667,7 +677,9 @@
* packet. This is not supported yet. */
LOGPENDP(endp, DRTP, LOGL_ERROR,
"cannot figure out how to convert RTP packet\n");
+ return -ENOTSUP;
}
+ return 0;
}
/* For AMR RTP two framing modes are defined RFC3267. There is a bandwith
@@ -685,7 +697,11 @@
unsigned int payload_len;
int rc;
- OSMO_ASSERT(*len >= sizeof(struct rtp_hdr));
+ if (*len < sizeof(struct rtp_hdr)) {
+ LOGPENDP(endp, DRTP, LOGL_ERROR, "AMR RTP packet too short (%d
< %zu)\n", *len, sizeof(struct rtp_hdr));
+ return -EINVAL;
+ }
+
rtp_hdr = (struct rtp_hdr *)data;
payload_len = *len - sizeof(struct rtp_hdr);
@@ -736,17 +752,21 @@
/* Check if a given RTP with AMR payload for octet-aligned mode */
-static bool amr_oa_check(char *data, int len)
+static int amr_oa_check(char *data, int len)
{
struct rtp_hdr *rtp_hdr;
unsigned int payload_len;
- OSMO_ASSERT(len >= sizeof(struct rtp_hdr));
+ if (len < sizeof(struct rtp_hdr))
+ return -EINVAL;
+
rtp_hdr = (struct rtp_hdr *)data;
payload_len = len - sizeof(struct rtp_hdr);
+ if (payload_len < sizeof(struct amr_hdr))
+ return -EINVAL;
- return osmo_amr_is_oa(rtp_hdr->data, payload_len);
+ return osmo_amr_is_oa(rtp_hdr->data, payload_len) ? 1 : 0;
}
/* Forward data to a debug tap. This is debug function that is intended for
@@ -1340,7 +1360,10 @@
* define, then we check if the incoming payload matches that
* expectation. */
if (amr_oa_bwe_convert_indicated(conn_src->end.codec)) {
- if (amr_oa_check(buf, len) !=
conn_src->end.codec->param.amr_octet_aligned)
+ int oa = amr_oa_check(buf, len);
+ if (oa < 0)
+ return -1;
+ if (((bool)oa) != conn_src->end.codec->param.amr_octet_aligned)
return -1;
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/15242
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
Gerrit-Change-Number: 15242
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-MessageType: newchange
