From c366717e66b6318c10a24ec10eede2c582a42b89 Mon Sep 17 00:00:00 2001
From: Simon Kelley <simon@thekelleys.org.uk>
Date: Fri, 13 Oct 2017 23:26:29 +0100
Subject: [PATCH] Tidy up add_resource_record() buffer size checks.

Mainly code-size and readability fixes.

Also return NULL from do_rfc1035_name() when limit exceeded, so
that truncated bit gets set in answer.
---
 src/rfc1035.c | 52 ++++++++++++++++++++-------------------------------
 src/util.c    | 12 ++++++++----
 2 files changed, 28 insertions(+), 36 deletions(-)

diff --git a/src/rfc1035.c b/src/rfc1035.c
index 56ab88b..22d94b4 100644
--- a/src/rfc1035.c
+++ b/src/rfc1035.c
@@ -1074,18 +1074,15 @@ 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;
+  if (limit && p + (size) > (unsigned char*)limit) goto truncated;
 
   va_start(ap, format);   /* make ap point to 1st unamed argument */
-
+  
+  if (truncp && *truncp)
+    goto truncated;
+  
   if (nameoffset > 0)
     {
       CHECK_LIMIT(2);
@@ -1095,10 +1092,7 @@ 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, limit)))
-	{
-	  va_end(ap);
-	  goto truncated;
-	}
+	goto truncated;
       
       if (nameoffset < 0)
 	{
@@ -1163,13 +1157,9 @@ int add_resource_record(struct dns_header *header, char *limit, int *truncp, int
         /* 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);
+        if (!(p = do_rfc1035_name(p, va_arg(ap, char *), limit)))
+	  goto truncated;
+	CHECK_LIMIT(1);
         *p++ = 0;
 	break;
 	
@@ -1194,24 +1184,22 @@ int add_resource_record(struct dns_header *header, char *limit, int *truncp, int
 	break;
       }
 
-#undef CHECK_LIMIT
   va_end(ap);	/* clean up variable argument pointer */
   
+  /* Now, store real RDLength. sav already checked against limit. */
   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;
-    }
+  PUTSHORT(j, sav);
   
   *pp = p;
   return 1;
+  
+ truncated:
+  va_end(ap);
+  if (truncp)
+    *truncp = 1;
+  return 0;
+
+#undef CHECK_LIMIT
 }
 
 static unsigned long crec_ttl(struct crec *crecp, time_t now)
diff --git a/src/util.c b/src/util.c
index 2a1d448..f5d4a78 100644
--- a/src/util.c
+++ b/src/util.c
@@ -246,14 +246,16 @@ unsigned char *do_rfc1035_name(unsigned char *p, char *sval, char *limit)
   
   while (sval && *sval)
     {
-      if (limit && p + 1 > (unsigned char*)limit)
-        return p;
-
       unsigned char *cp = p++;
+
+      if (limit && p > (unsigned char*)limit)
+        return NULL;
+
       for (j = 0; *sval && (*sval != '.'); sval++, j++)
 	{
           if (limit && p + 1 > (unsigned char*)limit)
-            return p;
+            return NULL;
+
 #ifdef HAVE_DNSSEC
 	  if (option_bool(OPT_DNSSEC_VALID) && *sval == NAME_ESCAPE)
 	    *p++ = (*(++sval))-1;
@@ -261,10 +263,12 @@ unsigned char *do_rfc1035_name(unsigned char *p, char *sval, char *limit)
 #endif		
 	    *p++ = *sval;
 	}
+      
       *cp  = j;
       if (*sval)
 	sval++;
     }
+  
   return p;
 }