This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
dexter gerrit-no-reply at lists.osmocom.orgdexter has uploaded this change for review. ( https://gerrit.osmocom.org/10330 Change subject: network: check packets before further processing ...................................................................... network: check packets before further processing When we receive a packet, we do not really check the contents. However, we should at least do some basic checks. - Check for short RTP packets - Check if the length field of RTCP packets seems plausible - Check if the packet type of RTCP packets makes sense (IANA) Change-Id: Id47b9eee2164c542e6b673db24974859dd0a7618 Related: OS#3444 --- M src/libosmo-mgcp/mgcp_network.c 1 file changed, 69 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/30/10330/1 diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 6dfc5a5..427f3c6 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -32,6 +32,7 @@ #include <osmocom/core/msgb.h> #include <osmocom/core/select.h> #include <osmocom/core/socket.h> +#include <osmocom/core/byteswap.h> #include <osmocom/netif/rtp.h> #include <osmocom/mgcp/mgcp.h> #include <osmocom/mgcp/mgcp_common.h> @@ -939,6 +940,53 @@ return 0; } +/* Do some basic checks to make sure that the RTCP packets we are going to + * process are not complete garbage */ +static int check_rtcp(char *buf, unsigned int buf_size) +{ + struct rtcp_hdr *hdr; + unsigned int len; + uint8_t type; + + /* RTPC packets that are just a header without data do not make + * any sense. */ + if (buf_size < sizeof(struct rtcp_hdr)) + return -EINVAL; + + /* Make sure that the length of the received packet does not exceed + * the available buffer size */ + hdr = (struct rtcp_hdr *)buf; + len = (osmo_ntohs(hdr->length) + 1) * 4; + if (len > buf_size) + return -EINVAL; + + /* Make sure we accept only packets that have a proper packet type set + * See also: http://www.iana.org/assignments/rtp-parameters/rtp-parameters.xhtml */ + type = hdr->type; + if ((type < 192 || type > 195) && (type < 200 || type > 213)) + return -EINVAL; + + return 0; +} + +/* Do some basic checks to make sure that the RTP packets we are going to + * process are not complete garbage */ +static int check_rtp(char *buf, unsigned int buf_size) +{ + /* RTP packets that are just a header without data do not make + * any sense. */ + if (buf_size < sizeof(struct rtp_hdr)) + return -EINVAL; + + /* FIXME: Add more checks, the reason why we do not check more than + * the length is because we currently handle IUUP packets as RTP + * packets, so they must pass this check, if we weould be more + * strict here, we would possibly break 3G. (see also FIXME note + * below */ + + return 0; +} + /* Receive RTP data from a specified source connection and dispatch it to a * destination connection. */ static int mgcp_recv(int *proto, struct sockaddr_in *addr, char *buf, @@ -959,8 +1007,29 @@ rc = receive_from(endp, fd->fd, addr, buf, buf_size); if (rc <= 0) return -1; + + /* FIXME: The way how we detect the protocol looks odd. We should look + * into the packet header. Also we should introduce a packet type + * MGCP_PROTO_IUUP because currently we handle IUUP packets like RTP + * packets which is problematic. */ *proto = fd == &conn->end.rtp ? MGCP_PROTO_RTP : MGCP_PROTO_RTCP; + if (*proto == MGCP_PROTO_RTP) { + if (check_rtp(buf, rc) < 0) { + LOGP(DRTP, LOGL_ERROR, + "endpoint:0x%x invalid RTP packet received -- packet tossed\n", + ENDPOINT_NUMBER(endp)); + return -1; + } + } else if (*proto == MGCP_PROTO_RTCP) { + if (check_rtcp(buf, rc) < 0) { + LOGP(DRTP, LOGL_ERROR, + "endpoint:0x%x invalid RTCP packet received -- packet tossed\n", + ENDPOINT_NUMBER(endp)); + return -1; + } + } + LOGP(DRTP, LOGL_DEBUG, "endpoint:0x%x ", ENDPOINT_NUMBER(endp)); LOGPC(DRTP, LOGL_DEBUG, "receiving from %s %s %d\n", conn->conn->name, inet_ntoa(addr->sin_addr), -- To view, visit https://gerrit.osmocom.org/10330 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id47b9eee2164c542e6b673db24974859dd0a7618 Gerrit-Change-Number: 10330 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier at sysmocom.de> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180803/60173e39/attachment.htm>