mirror of
https://gitea.osmocom.org/cellular-infrastructure/osmo-mgw.git
synced 2025-11-02 21:13:44 +00:00
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 some return values to be able to distinguish successful operation from invalid RTP sizes. In rtp_data_net(), make sure to return negative if the RTP packet was invalid. Some of the error return codes implemented here will only be used in upcoming patch Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c. Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
This commit is contained in:
@@ -475,7 +475,9 @@ static int mgcp_patch_pt(struct mgcp_conn_rtp *conn_src,
|
|||||||
uint8_t pt_in;
|
uint8_t pt_in;
|
||||||
int pt_out;
|
int pt_out;
|
||||||
|
|
||||||
OSMO_ASSERT(len >= sizeof(struct rtp_hdr));
|
if (len < sizeof(struct rtp_hdr))
|
||||||
|
return -EINVAL;
|
||||||
|
|
||||||
rtp_hdr = (struct rtp_hdr *)data;
|
rtp_hdr = (struct rtp_hdr *)data;
|
||||||
|
|
||||||
pt_in = rtp_hdr->payload_type;
|
pt_in = rtp_hdr->payload_type;
|
||||||
@@ -640,8 +642,9 @@ void mgcp_patch_and_count(struct mgcp_endpoint *endp,
|
|||||||
/* There are different dialects used to format and transfer voice data. When
|
/* There are different dialects used to format and transfer voice data. When
|
||||||
* the receiving end expects GSM-HR data to be formated after RFC 5993, this
|
* 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
|
* function is used to convert between RFC 5993 and TS 101318, which we normally
|
||||||
* use. */
|
* use.
|
||||||
static void rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int *len)
|
* Return 0 on sucess, negative on errors like invalid data length. */
|
||||||
|
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
|
/* NOTE: *data has an overall length of RTP_BUF_SIZE, so there is
|
||||||
* plenty of space available to store the slightly larger, converted
|
* plenty of space available to store the slightly larger, converted
|
||||||
@@ -649,7 +652,12 @@ static void rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int *len)
|
|||||||
|
|
||||||
struct rtp_hdr *rtp_hdr;
|
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;
|
rtp_hdr = (struct rtp_hdr *)data;
|
||||||
|
|
||||||
if (*len == GSM_HR_BYTES + sizeof(struct rtp_hdr)) {
|
if (*len == GSM_HR_BYTES + sizeof(struct rtp_hdr)) {
|
||||||
@@ -667,7 +675,9 @@ static void rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int *len)
|
|||||||
* packet. This is not supported yet. */
|
* packet. This is not supported yet. */
|
||||||
LOGPENDP(endp, DRTP, LOGL_ERROR,
|
LOGPENDP(endp, DRTP, LOGL_ERROR,
|
||||||
"cannot figure out how to convert RTP packet\n");
|
"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
|
/* For AMR RTP two framing modes are defined RFC3267. There is a bandwith
|
||||||
@@ -685,7 +695,11 @@ static int amr_oa_bwe_convert(struct mgcp_endpoint *endp, char *data, int *len,
|
|||||||
unsigned int payload_len;
|
unsigned int payload_len;
|
||||||
int rc;
|
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;
|
rtp_hdr = (struct rtp_hdr *)data;
|
||||||
|
|
||||||
payload_len = *len - sizeof(struct rtp_hdr);
|
payload_len = *len - sizeof(struct rtp_hdr);
|
||||||
@@ -735,18 +749,23 @@ static bool amr_oa_bwe_convert_indicated(struct mgcp_rtp_codec *codec)
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
/* Check if a given RTP with AMR payload for octet-aligned mode */
|
/* Return whether an RTP packet with AMR payload is in octet-aligned mode.
|
||||||
static bool amr_oa_check(char *data, int len)
|
* Return 0 if in bandwidth-efficient mode, 1 for octet-aligned mode, and negative if the RTP data is invalid. */
|
||||||
|
static int amr_oa_check(char *data, int len)
|
||||||
{
|
{
|
||||||
struct rtp_hdr *rtp_hdr;
|
struct rtp_hdr *rtp_hdr;
|
||||||
unsigned int payload_len;
|
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;
|
rtp_hdr = (struct rtp_hdr *)data;
|
||||||
|
|
||||||
payload_len = len - sizeof(struct rtp_hdr);
|
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
|
/* Forward data to a debug tap. This is debug function that is intended for
|
||||||
@@ -1340,7 +1359,10 @@ static int rtp_data_net(struct osmo_fd *fd, unsigned int what)
|
|||||||
* define, then we check if the incoming payload matches that
|
* define, then we check if the incoming payload matches that
|
||||||
* expectation. */
|
* expectation. */
|
||||||
if (amr_oa_bwe_convert_indicated(conn_src->end.codec)) {
|
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;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user