From 2c415e570aed0b955698238a274a50f3ffa4892e Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Mon, 5 Oct 2020 18:37:47 +0800 Subject: dnsmasq: fix heap overflow Backport CVE-2017-14491. (http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=0549c73b7ea6b22a3c49beb4d432f185a81efcbc) Bug: 158221622 Test: 1. Manual testing - build dnsmasq with asan enabled and test the corresponding functionalities. 2. Test tethering still worked fine Change-Id: I90390535198fe0a7b8084879d39f813d8de51ab3 (cherry picked from commit b7ae3453bfcc139be75c8dec5cdb8a1453eb6e16) --- src/dnsmasq.h | 2 +- src/rfc1035.c | 28 +++++++++++++++++++++++++--- src/rfc2131.c | 4 ++-- src/util.c | 11 +++++++++-- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 2c5f964..9af3a2c 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -684,7 +684,7 @@ void rand_init(void); unsigned short rand16(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/rfc1035.c b/src/rfc1035.c index 0f883e1..47479ab 100644 --- a/src/rfc1035.c +++ b/src/rfc1035.c @@ -847,9 +847,19 @@ static int add_resource_record(HEADER* header, char* limit, int* truncp, unsigne 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); + PUTSHORT(nameoffset | 0xc000, p); PUTSHORT(type, p); PUTSHORT(class, p); @@ -858,11 +868,10 @@ static int add_resource_record(HEADER* header, char* limit, int* truncp, unsigne sav = p; /* Save pointer to RDLength field */ PUTSHORT(0, p); /* Placeholder RDLength */ - va_start(ap, format); /* make ap point to 1st unamed argument */ - for (; *format; format++) switch (*format) { #ifdef HAVE_IPV6 case '6': + CHECK_LIMIT(IN6ADDRSZ); sval = va_arg(ap, char*); memcpy(p, sval, IN6ADDRSZ); p += IN6ADDRSZ; @@ -870,17 +879,20 @@ static int add_resource_record(HEADER* header, char* limit, int* truncp, unsigne #endif case '4': + CHECK_LIMIT(INADDRSZ); sval = va_arg(ap, char*); memcpy(p, sval, INADDRSZ); p += INADDRSZ; 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; @@ -888,12 +900,18 @@ static int add_resource_record(HEADER* header, char* limit, int* truncp, unsigne 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 = 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*); memcpy(p, sval, usval); p += usval; @@ -903,19 +921,23 @@ static int add_resource_record(HEADER* header, char* limit, int* truncp, unsigne sval = va_arg(ap, char*); 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; + /* 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 f74ecb9..c0f649a 100644 --- a/src/rfc2131.c +++ b/src/rfc2131.c @@ -1889,8 +1889,8 @@ static void do_options(struct dhcp_context* context, struct dhcp_packet* mess, u *(p++) = 255; if (fqdn_flags & 0x04) { - p = do_rfc1035_name(p, hostname); - if (domain) p = do_rfc1035_name(p, domain); + p = do_rfc1035_name(p, hostname, NULL); + if (domain) p = do_rfc1035_name(p, domain, NULL); *p++ = 0; } else { memcpy(p, hostname, strlen(hostname)); diff --git a/src/util.c b/src/util.c index 53e3137..11b43a0 100644 --- a/src/util.c +++ b/src/util.c @@ -190,12 +190,19 @@ 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++) *p++ = *sval; + for (j = 0; *sval && (*sval != '.'); sval++, j++) { + if (limit && p + 1 > (unsigned char*)limit) + return p; + *p++ = *sval; + } *cp = j; if (*sval) sval++; } -- cgit v1.2.3 From 303ca2733ad5d3994cd728dc09f0cd2d2417b4f3 Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Tue, 8 Dec 2020 23:51:54 +0800 Subject: backport: dnsmasq: fix heap overflow Backport CVE-2017-14491. (http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=0549c73b7ea6b22a3c49beb4d432f185a81efcbc) Bug: 158221622 Test: 1. Manual testing - build dnsmasq with asan enabled and test the corresponding functionalities. 2. Test tethering still worked fine Change-Id: I05f33d872f44b015706139f0a438171c42b0b27e Merged-In: I90390535198fe0a7b8084879d39f813d8de51ab3 --- src/dnsmasq.h | 2 +- src/rfc1035.c | 28 +++++++++++++++++++++++++--- src/rfc2131.c | 4 ++-- src/util.c | 11 ++++++++--- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 2a78497..c10717e 100755 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -701,7 +701,7 @@ void rand_init(void); unsigned short rand16(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/rfc1035.c b/src/rfc1035.c index e440e8a..7d22448 100755 --- a/src/rfc1035.c +++ b/src/rfc1035.c @@ -1030,10 +1030,20 @@ static int add_resource_record(HEADER *header, char *limit, int *truncp, unsigne 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); + PUTSHORT(nameoffset | 0xc000, p); PUTSHORT(type, p); PUTSHORT(class, p); @@ -1041,14 +1051,13 @@ static int add_resource_record(HEADER *header, char *limit, int *truncp, unsigne sav = p; /* Save pointer to RDLength field */ PUTSHORT(0, p); /* Placeholder RDLength */ - - va_start(ap, format); /* make ap point to 1st unamed argument */ for (; *format; format++) switch (*format) { #ifdef HAVE_IPV6 case '6': + CHECK_LIMIT(IN6ADDRSZ); sval = va_arg(ap, char *); memcpy(p, sval, IN6ADDRSZ); p += IN6ADDRSZ; @@ -1056,17 +1065,20 @@ static int add_resource_record(HEADER *header, char *limit, int *truncp, unsigne #endif case '4': + CHECK_LIMIT(IN6ADDRSZ); sval = va_arg(ap, char *); memcpy(p, sval, INADDRSZ); p += INADDRSZ; 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; @@ -1075,12 +1087,18 @@ static int add_resource_record(HEADER *header, char *limit, int *truncp, unsigne /* 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 = 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 *); memcpy(p, sval, usval); p += usval; @@ -1091,20 +1109,24 @@ static int add_resource_record(HEADER *header, char *limit, int *truncp, unsigne 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; + /* 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 1ef8569..0ce107d 100755 --- a/src/rfc2131.c +++ b/src/rfc2131.c @@ -2188,9 +2188,9 @@ 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; } else diff --git a/src/util.c b/src/util.c index e44b8fa..687804a 100755 --- a/src/util.c +++ b/src/util.c @@ -206,15 +206,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++) - *p++ = *sval; + for (j = 0; *sval && (*sval != '.'); sval++, j++) { + if (limit && p + 1 > (unsigned char*)limit) + return p; + *p++ = *sval; + } *cp = j; if (*sval) sval++; -- cgit v1.2.3