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.stegl...@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

<<attachment: daniel_steglich.vcf>>

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Reply via email to