From 8868a04895b27d42d42e364f1a0c0196c1505b04 Mon Sep 17 00:00:00 2001
From: Simon Kelley <simon@thekelleys.org.uk>
Date: Mon, 25 Sep 2017 18:17:11 +0100
Subject: [PATCH 1/9] Security fix, CVE-2017-14491 DNS heap buffer
overflow.
Fix heap overflow in DNS code. This is a potentially serious
security hole. It allows an attacker who can make DNS
requests to dnsmasq, and who controls the contents of
a domain, which is thereby queried, to overflow
(by 2 bytes) a heap buffer and either crash, or
even take control of, dnsmasq.
---
src/dnsmasq.h | 2 +-
src/dnssec.c | 2 +-
src/option.c | 2 +-
src/rfc1035.c | 50 +++++++++++++++++++++++++++++++++++++++++---------
src/rfc2131.c | 4 ++--
src/rfc3315.c | 4 ++--
src/util.c | 7 ++++++-
7 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 1179492..06e5579 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1162,7 +1162,7 @@ u32 rand32(void);
u64 rand64(void);
int legal_hostname(char *c);
char *canonicalise(char *s, int *nomem);
-unsigned char *do_rfc1035_name(unsigned char *p, char *sval);
+unsigned char *do_rfc1035_name(unsigned char *p, char *sval, char *limit);
void *safe_malloc(size_t size);
void safe_pipe(int *fd, int read_noblock);
void *whine_malloc(size_t size);
diff --git a/src/dnssec.c b/src/dnssec.c
index 3c77c7d..f45c804 100644
--- a/src/dnssec.c
+++ b/src/dnssec.c
@@ -2227,7 +2227,7 @@ size_t dnssec_generate_query(struct dns_header *header, unsigned char *end, char
p = (unsigned char *)(header+1);
- p = do_rfc1035_name(p, name);
+ p = do_rfc1035_name(p, name, NULL);
*p++ = 0;
PUTSHORT(type, p);
PUTSHORT(class, p);
diff --git a/src/option.c b/src/option.c
index eb78b1a..3469f53 100644
--- a/src/option.c
+++ b/src/option.c
@@ -1378,7 +1378,7 @@ static int parse_dhcp_opt(char *errstr, char *arg, int flags)
}
p = newp;
- end = do_rfc1035_name(p + len, dom);
+ end = do_rfc1035_name(p + len, dom, NULL);
*end++ = 0;
len = end - p;
free(dom);
diff --git a/src/rfc1035.c b/src/rfc1035.c
index 24d08c1..78410d6 100644
--- a/src/rfc1035.c
+++ b/src/rfc1035.c
@@ -1049,6 +1049,7 @@ int check_for_ignored_address(struct dns_header *header, size_t qlen, struct bog
return 0;
}
+
int add_resource_record(struct dns_header *header, char *limit, int *truncp, int nameoffset, unsigned char **pp,
unsigned long ttl, int *offset, unsigned short type, unsigned short class, char *format, ...)
{
@@ -1058,12 +1059,21 @@ int add_resource_record(struct dns_header *header, char *limit, int *truncp, int
unsigned short usval;
long lval;
char *sval;
+#define CHECK_LIMIT(size) \
+ if (limit && p + (size) > (unsigned char*)limit) \
+ { \
+ va_end(ap); \
+ goto truncated; \
+ }
if (truncp && *truncp)
return 0;
-
+
va_start(ap, format); /* make ap point to 1st unamed argument */
-
+
+ /* nameoffset (1 or 2) + type (2) + class (2) + ttl (4) + 0 (2) */
+ CHECK_LIMIT(12);
+
if (nameoffset > 0)
{
PUTSHORT(nameoffset | 0xc000, p);
@@ -1072,7 +1082,13 @@ int add_resource_record(struct dns_header *header, char *limit, int *truncp, int
{
char *name = va_arg(ap, char *);
if (name)
- p = do_rfc1035_name(p, name);
+ p = do_rfc1035_name(p, name, limit);
+ if (!p)
+ {
+ va_end(ap);
+ goto truncated;
+ }
+
if (nameoffset < 0)
{
PUTSHORT(-nameoffset | 0xc000, p);
@@ -1093,6 +1109,7 @@ int add_resource_record(struct dns_header *header, char *limit, int *truncp, int
{
#ifdef HAVE_IPV6
case '6':
+ CHECK_LIMIT(IN6ADDRSZ);
sval = va_arg(ap, char *);
memcpy(p, sval, IN6ADDRSZ);
p += IN6ADDRSZ;
@@ -1100,36 +1117,47 @@ int add_resource_record(struct dns_header *header, char *limit, int *truncp, int
#endif
case '4':
+ CHECK_LIMIT(INADDRSZ);
sval = va_arg(ap, char *);
memcpy(p, sval, INADDRSZ);
p += INADDRSZ;
break;
case 'b':
+ CHECK_LIMIT(1);
usval = va_arg(ap, int);
*p++ = usval;
break;
case 's':
+ CHECK_LIMIT(2);
usval = va_arg(ap, int);
PUTSHORT(usval, p);
break;
case 'l':
+ CHECK_LIMIT(4);
lval = va_arg(ap, long);
PUTLONG(lval, p);
break;
case 'd':
- /* get domain-name answer arg and store it in RDATA field */
- if (offset)
- *offset = p - (unsigned char *)header;
- p = do_rfc1035_name(p, va_arg(ap, char *));
- *p++ = 0;
+ /* get domain-name answer arg and store it in RDATA field */
+ if (offset)
+ *offset = p - (unsigned char *)header;
+ p = do_rfc1035_name(p, va_arg(ap, char *), limit);
+ if (!p)
+ {
+ va_end(ap);
+ goto truncated;
+ }
+ CHECK_LIMIT(1);
+ *p++ = 0;
break;
case 't':
usval = va_arg(ap, int);
+ CHECK_LIMIT(usval);
sval = va_arg(ap, char *);
if (usval != 0)
memcpy(p, sval, usval);
@@ -1141,20 +1169,24 @@ int add_resource_record(struct dns_header *header, char *limit, int *truncp, int
usval = sval ? strlen(sval) : 0;
if (usval > 255)
usval = 255;
+ CHECK_LIMIT(usval + 1);
*p++ = (unsigned char)usval;
memcpy(p, sval, usval);
p += usval;
break;
}
+#undef CHECK_LIMIT
va_end(ap); /* clean up variable argument pointer */
j = p - sav - 2;
- PUTSHORT(j, sav); /* Now, store real RDLength */
+ /* this has already been checked against limit before */
+ PUTSHORT(j, sav); /* Now, store real RDLength */
/* check for overflow of buffer */
if (limit && ((unsigned char *)limit - p) < 0)
{
+truncated:
if (truncp)
*truncp = 1;
return 0;
diff --git a/src/rfc2131.c b/src/rfc2131.c
index 8b99d4b..75893a6 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -2420,10 +2420,10 @@ static void do_options(struct dhcp_context *context,
if (fqdn_flags & 0x04)
{
- p = do_rfc1035_name(p, hostname);
+ p = do_rfc1035_name(p, hostname, NULL);
if (domain)
{
- p = do_rfc1035_name(p, domain);
+ p = do_rfc1035_name(p, domain, NULL);
*p++ = 0;
}
}
diff --git a/src/rfc3315.c b/src/rfc3315.c
index 3f4d69c..73bdee4 100644
--- a/src/rfc3315.c
+++ b/src/rfc3315.c
@@ -1472,10 +1472,10 @@ static struct dhcp_netid *add_options(struct state *state, int do_refresh)
if ((p = expand(len + 2)))
{
*(p++) = state->fqdn_flags;
- p = do_rfc1035_name(p, state->hostname);
+ p = do_rfc1035_name(p, state->hostname, NULL);
if (state->send_domain)
{
- p = do_rfc1035_name(p, state->send_domain);
+ p = do_rfc1035_name(p, state->send_domain, NULL);
*p = 0;
}
}
diff --git a/src/util.c b/src/util.c
index 1a9f228..be9f8a6 100644
--- a/src/util.c
+++ b/src/util.c
@@ -218,15 +218,20 @@ char *canonicalise(char *in, int *nomem)
return ret;
}
-unsigned char *do_rfc1035_name(unsigned char *p, char *sval)
+unsigned char *do_rfc1035_name(unsigned char *p, char *sval, char *limit)
{
int j;
while (sval && *sval)
{
+ if (limit && p + 1 > (unsigned char*)limit)
+ return p;
+
unsigned char *cp = p++;
for (j = 0; *sval && (*sval != '.'); sval++, j++)
{
+ if (limit && p + 1 > (unsigned char*)limit)
+ return p;
#ifdef HAVE_DNSSEC
if (option_bool(OPT_DNSSEC_VALID) && *sval == NAME_ESCAPE)
*p++ = (*(++sval))-1;
--
2.9.5