[Dnsmasq-discuss] Backport fix for CVE-2017-14491 to dnsmasq v2.57

Daniel Steglich daniel.steglich at sphairon.de
Tue Oct 10 07:24:03 BST 2017


Hi,

we have some legacy devices in the field which makes use of dnsmasq.
Of course our recent products will get the update to dnsmasq 2.78 but in order to provide a minimal patch to fix the latest CVE's we backported the security patch to 2.57.

The legacy devices I'm talking about are not even IPv6 capable, so I excluded the IPv6 (DHCPv6/SLAAC) related CVE's and only created a backport patch for the CVE-2017-14491.

Recently I saw a patch popping up for debian wheezy where dnsmasq 2.62 is still in use.
The version we are using in our legacy products is 2.57 and therefore is pretty close to the debian version.

I already compared our backport patch with the debian one and it looks pretty similar. But I'm wondering if there is any source for such security related backport patches for dnsmasq.

If anybody wants to use this backport patch feel free to do so. If anybody could comment on the patch and tell me if I missed something, I would appreciate.

Please note that the patch proposed by google initially also contains two more changes at rfc1035.c:

diff -u a/rfc1035.c b/rfc1035.c
--- a/rfc1035.c	2017-09-13 12:25:39.971673852 -0700
+++ b/rfc1035.c	2017-09-13 12:21:07.275689168 -0700
@@ -37,7 +37,7 @@
 	/* end marker */
 	{
 	  /* check that there are the correct no of bytes after the name */
-	  if (!CHECK_LEN(header, p, plen, extrabytes))
+	  if (!CHECK_LEN(header, p1 ? p1 : p, plen, extrabytes))
 	    return 0;

 	  if (isExtract)
@@ -498,6 +498,8 @@
 	    {
 	      unsigned int i, len = *p1;
 	      unsigned char *p2 = p1;
+	      if ((p1 + len - p) >= rdlen)
+	        return 0; /* bad packet */
 	      /* make counted string zero-term  and sanitise */

 	      for (i = 0; i < len; i++)
 		{

Also the patch does not propose to move the va_start() function call up. I adapted our backport to this changes after I had a look into the debian patches because I saw the debian patch was supplied
by Simon Kelly and he probably knows his code better then me.

Cheers,

Daniel




From cc0e1d567766bd8aed7e4c66272546b6f54c09ab Mon Sep 17 00:00:00 2001
From: Daniel Steglich <daniel.steglich at sphairon.com>
Date: Mon, 9 Oct 2017 15:41:58 +0200
Subject: [PATCH] Backport of debian patch for CVE-2017-14491

Backported debian patch for CVE-2017-14491 to be able to apply
this patch on dnsmasq version 2.57.
---
 src/dnsmasq.h |  2 +-
 src/rfc1035.c | 42 ++++++++++++++++++++++++++++++---------
 src/rfc2131.c |  4 ++--
 src/util.c    |  8 +++++++-
 4 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 76eab3f..ceb35bb 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -775,7 +775,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 889c1f0..47e68fe 100644
--- a/src/rfc1035.c
+++ b/src/rfc1035.c
@@ -1185,9 +1185,21 @@ static int add_resource_record(struct dns_header *header, char *limit, int *trun
   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);
@@ -1196,13 +1208,12 @@ static int add_resource_record(struct dns_header *header, char *limit, int *trun
   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;
@@ -1210,31 +1221,41 @@ static int add_resource_record(struct dns_header *header, char *limit, int *trun
 #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;
 	
       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 *);
 	memcpy(p, sval, usval);
 	p += usval;
@@ -1245,20 +1266,23 @@ static int add_resource_record(struct dns_header *header, char *limit, int *trun
 	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 e8e6562..68ce1b6 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -2329,9 +2329,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 e64f1a6..8384423 100644
--- a/src/util.c
+++ b/src/util.c
@@ -202,15 +202,21 @@ 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;
 	*p++ = *sval;
+      }
       *cp  = j;
       if (*sval)
 	sval++;
-- 
2.7.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: daniel_steglich.vcf
Type: text/x-vcard
Size: 448 bytes
Desc: not available
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20171010/eeb4999e/attachment.vcf>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20171010/eeb4999e/attachment.sig>


More information about the Dnsmasq-discuss mailing list