From b73bdf321a1c14c0c593fb65fc1c002cd8b41b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Wed, 10 Aug 2022 22:48:51 +0200 Subject: [PATCH] asymcute: fix one byte out-of-bounds access in _len_get As per Section 5.2.1 of the MQTT-SN specification, the MQTT-SN length header is either 1- or 3-octet long. If it is 3-octet long then the first octet is 0x01. The asymcute implementation currently only checks that the incoming packet is at least 2-octet long before attempting to parse it (MIN_PKT_LEN). However, if the first octet is 0x01 the packet must be more than 3 octet long in order to be valid. Since asymcute does not check this it reads one octet beyond the packet data for a 2-octet packet where the first octet has the value 0x01. This commit fixes this issue by adding an additional sanity check to _len_get. --- sys/net/application_layer/asymcute/asymcute.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/sys/net/application_layer/asymcute/asymcute.c b/sys/net/application_layer/asymcute/asymcute.c index aad730906f7a..6792fba75466 100644 --- a/sys/net/application_layer/asymcute/asymcute.c +++ b/sys/net/application_layer/asymcute/asymcute.c @@ -83,13 +83,16 @@ static size_t _len_set(uint8_t *buf, size_t len) } } -static size_t _len_get(uint8_t *buf, size_t *len) +static ssize_t _len_get(uint8_t *buf, size_t pkt_len, size_t *len) { if (buf[0] != 0x01) { *len = (uint16_t)buf[0]; return 1; } else { + if (pkt_len < 3) { + return -1; + } *len = byteorder_bebuftohs(&buf[1]); return 3; } @@ -107,8 +110,10 @@ static uint16_t _msg_id_next(asymcute_con_t *con) static uint8_t _req_type(asymcute_req_t *req) { size_t len; - size_t pos = _len_get(req->data, &len); - return req->data[pos]; + ssize_t pos = _len_get(req->data, req->data_len, &len); + /* requests are created by us and should thus always be valid */ + assert(pos != -1 && (size_t)pos < req->data_len); + return req->data[(size_t)pos]; } /* @pre con is locked */ @@ -590,7 +595,12 @@ void _on_pkt(sock_udp_t *sock, sock_async_flags_t type, void *arg) CONFIG_ASYMCUTE_BUFSIZE, 0, NULL); if (pkt_len >= MIN_PKT_LEN) { size_t len; - size_t pos = _len_get(con->rxbuf, &len); + ssize_t lret = _len_get(con->rxbuf, pkt_len, &len); + if (lret == -1) { + /* first octet was 0x01 but pkt does not have more than 3 octets */ + return; + } + size_t pos = (size_t)lret; /* validate incoming data: verify message length */ if (((size_t)pkt_len <= pos) || ((size_t)pkt_len < len)) {