From ae367e5537eec6f5e513db57a11d8a689a2dad97 Mon Sep 17 00:00:00 2001 From: Hauke Petersen Date: Thu, 22 Jun 2017 13:53:12 +0200 Subject: [PATCH] net/emcute: check value of length field before use --- sys/include/net/emcute.h | 2 +- sys/net/application_layer/emcute/emcute.c | 31 +++++++++++++++++------ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/sys/include/net/emcute.h b/sys/include/net/emcute.h index b0c1eb9762..8afd647003 100644 --- a/sys/include/net/emcute.h +++ b/sys/include/net/emcute.h @@ -210,7 +210,7 @@ typedef struct { * @brief Signature for callbacks fired when publish messages are received * * @param[in] topic topic the received data was published on - * @param[in] data published data + * @param[in] data published data, can be NULL * @param[in] len length of @p data in bytes */ typedef void(*emcute_cb_t)(const emcute_topic_t *topic, void *data, size_t len); diff --git a/sys/net/application_layer/emcute/emcute.c b/sys/net/application_layer/emcute/emcute.c index 8279f84cae..1ea73b3c1d 100644 --- a/sys/net/application_layer/emcute/emcute.c +++ b/sys/net/application_layer/emcute/emcute.c @@ -165,21 +165,24 @@ static void on_ack(uint8_t type, int id_pos, int ret_pos, int res_pos) } } -static void on_publish(void) +static void on_publish(uint16_t len, int pos) { + /* make sure packet length is valid - if not, drop packet silently */ + if ((int)len < (pos + 6)) { + return; + } + emcute_sub_t *sub; - uint16_t len; - int pos = get_len(rbuf, &len); uint16_t tid = get_u16(&rbuf[pos + 2]); /* allocate a response packet */ uint8_t buf[7] = { 7, PUBACK, 0, 0, 0, 0, ACCEPT }; /* and populate message ID and topic ID fields */ - memcpy(&buf[2], &rbuf[3], 4); + memcpy(&buf[2], &rbuf[pos + 2], 4); /* return error code in case we don't support/understand active flags. So * far we only understand QoS 1... */ - if (rbuf[2] & ~(EMCUTE_QOS_1 | EMCUTE_TIT_SHORT)) { + if (rbuf[pos + 1] & ~(EMCUTE_QOS_1 | EMCUTE_TIT_SHORT)) { buf[6] = REJ_NOTSUP; sock_udp_send(&sock, &buf, 7, &gateway); return; @@ -193,11 +196,13 @@ static void on_publish(void) DEBUG("[emcute] on pub: no subscription found\n"); } else { - if (rbuf[2] & EMCUTE_QOS_1) { + if (rbuf[pos + 1] & EMCUTE_QOS_1) { sock_udp_send(&sock, &buf, 7, &gateway); } DEBUG("[emcute] on pub: got %i bytes of data\n", (int)(len - pos - 6)); - sub->cb(&sub->topic, &rbuf[pos + 6], (size_t)(len - pos - 6)); + size_t dat_len = (len - pos - 6); + void *dat = (dat_len > 0) ? &rbuf[pos + 6] : NULL; + sub->cb(&sub->topic, dat, dat_len); } } @@ -532,7 +537,17 @@ void emcute_run(uint16_t port, const char *id) if (len >= 2) { /* handle the packet */ uint16_t pkt_len; + /* catch invalid length field */ + if ((len == 2) && (rbuf[0] == 0x01)) { + continue; + } + /* parse length field */ int pos = get_len(rbuf, &pkt_len); + /* verify length to prevent overflows */ + if (((ssize_t)pkt_len > len) || ((ssize_t)pos >= len)) { + continue; + } + /* get packet type */ uint8_t type = rbuf[pos]; switch (type) { @@ -540,7 +555,7 @@ void emcute_run(uint16_t port, const char *id) case WILLTOPICREQ: on_ack(type, 0, 0, 0); break; case WILLMSGREQ: on_ack(type, 0, 0, 0); break; case REGACK: on_ack(type, 4, 6, 2); break; - case PUBLISH: on_publish(); break; + case PUBLISH: on_publish(len, pos); break; case PUBACK: on_ack(type, 4, 6, 0); break; case SUBACK: on_ack(type, 5, 7, 3); break; case UNSUBACK: on_ack(type, 2, 0, 0); break;