From 2840b3825eb6e0b7320fb111d06832ca9a9e3148 Mon Sep 17 00:00:00 2001
From: Martine Lenders <m.lenders@fu-berlin.de>
Date: Wed, 9 Jan 2019 19:32:46 +0100
Subject: [PATCH] sock_dns: fix out-of-bound errors
Fixes #10739
---
Makefile.dep | 1 +
sys/net/application_layer/dns/dns.c | 60 +++++++++++++++++++++++------
2 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/Makefile.dep b/Makefile.dep
index 3cecda907e..e324203ade 100644
--- a/Makefile.dep
+++ b/Makefile.dep
@@ -689,6 +689,7 @@ endif
ifneq (,$(filter sock_dns,$(USEMODULE)))
USEMODULE += sock_util
+ USEMODULE += posix
endif
ifneq (,$(filter sock_util,$(USEMODULE)))
diff --git a/sys/net/application_layer/dns/dns.c b/sys/net/application_layer/dns/dns.c
index adef0df244..8bda172367 100644
--- a/sys/net/application_layer/dns/dns.c
+++ b/sys/net/application_layer/dns/dns.c
@@ -15,6 +15,7 @@
* @}
*/
+#include <arpa/inet.h>
#include <string.h>
#include <stdio.h>
@@ -73,34 +74,58 @@ static unsigned _get_short(uint8_t *buf)
return _tmp;
}
-static size_t _skip_hostname(uint8_t *buf)
+static ssize_t _skip_hostname(const uint8_t *buf, size_t len, uint8_t *bufpos)
{
- uint8_t *bufpos = buf;
+ const uint8_t *buflim = buf + len;
+ unsigned res = 0;
+ if (bufpos >= buflim) {
+ /* out-of-bound */
+ return -EBADMSG;
+ }
/* handle DNS Message Compression */
if (*bufpos >= 192) {
+ if ((bufpos + 2) >= buflim) {
+ return -EBADMSG;
+ }
return 2;
}
- while(*bufpos) {
- bufpos += *bufpos + 1;
+ while (bufpos[res]) {
+ res += bufpos[res] + 1;
+ if ((&bufpos[res]) >= buflim) {
+ /* out-of-bound */
+ return -EBADMSG;
+ }
}
- return (bufpos - buf + 1);
+ return res + 1;
}
static int _parse_dns_reply(uint8_t *buf, size_t len, void* addr_out, int family)
{
+ const uint8_t *buflim = buf + len;
sock_dns_hdr_t *hdr = (sock_dns_hdr_t*) buf;
uint8_t *bufpos = buf + sizeof(*hdr);
/* skip all queries that are part of the reply */
for (unsigned n = 0; n < ntohs(hdr->qdcount); n++) {
- bufpos += _skip_hostname(bufpos);
+ ssize_t tmp = _skip_hostname(buf, len, bufpos);
+ if (tmp < 0) {
+ return tmp;
+ }
+ bufpos += tmp;
bufpos += 4; /* skip type and class of query */
}
for (unsigned n = 0; n < ntohs(hdr->ancount); n++) {
- bufpos += _skip_hostname(bufpos);
+ ssize_t tmp = _skip_hostname(buf, len, bufpos);
+ if (tmp < 0) {
+ return tmp;
+ }
+ bufpos += tmp;
+ if ((bufpos + 2 + 2 + 4) >= buflim) {
+ return -EBADMSG;
+ }
uint16_t _type = ntohs(_get_short(bufpos));
bufpos += 2;
uint16_t class = ntohs(_get_short(bufpos));
@@ -108,20 +133,31 @@ static int _parse_dns_reply(uint8_t *buf, size_t len, void* addr_out, int family
bufpos += 4; /* skip ttl */
unsigned addrlen = ntohs(_get_short(bufpos));
- bufpos += 2;
- if ((bufpos + addrlen) > (buf + len)) {
- return -EBADMSG;
- }
-
/* skip unwanted answers */
if ((class != DNS_CLASS_IN) ||
((_type == DNS_TYPE_A) && (family == AF_INET6)) ||
((_type == DNS_TYPE_AAAA) && (family == AF_INET)) ||
! ((_type == DNS_TYPE_A) || ((_type == DNS_TYPE_AAAA))
)) {
+ if (addrlen > len) {
+ /* buffer wraps around memory space */
+ return -EBADMSG;
+ }
bufpos += addrlen;
+ /* other out-of-bound is checked in `_skip_hostname()` at start of
+ * loop */
continue;
}
+ if (((addrlen != INADDRSZ) && (family == AF_INET)) ||
+ ((addrlen != IN6ADDRSZ) && (family == AF_INET6)) ||
+ ((addrlen != IN6ADDRSZ) && (addrlen != INADDRSZ) &&
+ (family == AF_UNSPEC))) {
+ return -EBADMSG;
+ }
+ bufpos += 2;
+ if ((bufpos + addrlen) >= buflim) {
+ return -EBADMSG;
+ }
memcpy(addr_out, bufpos, addrlen);
return addrlen;
--
GitLab