From e633a21b5f7afafa913843e16b687cdb674147e9 Mon Sep 17 00:00:00 2001
From: dmiller <dmiller@e0a8ed71-7df4-0310-8962-fdc924857419>
Date: Fri, 19 May 2023 01:43:17 +0000
Subject: [PATCH] Avoid recursion in DNS parsing; check label/name lengths

---
 nmap_dns.cc | 65 ++++++++++++++++++++++++++++++++++++++---------------
 nmap_dns.h  |  2 ++
 2 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/nmap_dns.cc b/nmap_dns.cc
index eb89dabe8..50c8a81e1 100644
--- a/nmap_dns.cc
+++ b/nmap_dns.cc
@@ -1519,40 +1519,69 @@ size_t DNS::Factory::parseUnsignedInt(u32 &num, const u8 *buf, size_t offset, si
 
 size_t DNS::Factory::parseDomainName(std::string &name, const u8 *buf, size_t offset, size_t maxlen)
 {
-  size_t tmp, ret = 0;
+  size_t tmp = 0;
+  size_t max_offset = offset;
+  size_t curr_offset = offset;
 
   name.clear();
-  while(u8 label_length = buf[offset+ret++]) // Postincrement important here
+  while(u8 label_length = buf[curr_offset])
   {
     if((label_length & COMPRESSED_NAME) == COMPRESSED_NAME)
     {
-      --ret; // The byte it's part of the pointer, wasn't really consumed yet
       u16 real_offset;
-      DNS_CHECK_ACCUMLATE(ret, tmp, parseUnsignedShort(real_offset, buf, offset+ret, maxlen));
-      real_offset -= COMPRESSED_NAME<<8;
-      if( real_offset < offset)
-      {
-        std::string val;
-        DNS_CHECK_ACCUMLATE(tmp, tmp, parseDomainName(val, buf, real_offset, maxlen));
-        name+=val;
-        return ret;
+      tmp = parseUnsignedShort(real_offset, buf, curr_offset, maxlen);
+      if (tmp < 1) {
+        return 0;
+      }
+      if (curr_offset >= max_offset) {
+        max_offset = curr_offset + tmp;
+      }
+      real_offset -= COMPRESSED_NAME<<8;
+      if(real_offset < curr_offset)
+      {
+        curr_offset = real_offset;
+        continue;
+      }
+      else {
+        if (o.debugging) {
+          log_write(LOG_STDOUT, "DNS compression pointer is not backwards\n");
+        }
+        return 0;
       }
-      else return 0;
     }
 
-    for(u8 i=0; i<label_length; ++i)
-    {
-      size_t index = offset+ret++;  // Postincrement important here
-      DNS_CHECK_UPPER_BOUND(index, maxlen);
-      name += buf[index];
+    if (label_length > DNS_LABEL_MAX_LENGTH) {
+      if (o.debugging) {
+        log_write(LOG_STDOUT, "DNS label exceeds max length\n");
+      }
+      return 0;
+    }
+
+    curr_offset++;
+    DNS_CHECK_UPPER_BOUND(curr_offset + label_length, maxlen);
+    name.append(reinterpret_cast<const char *>(buf + curr_offset), label_length);
+    curr_offset += label_length;
+    if (curr_offset > max_offset) {
+      max_offset = curr_offset;
     }
     name += '.';
+
+    if (name.length() > DNS_NAME_MAX_LENGTH - 1) {
+      if (o.debugging) {
+        log_write(LOG_STDOUT, "DNS name exceeds max length\n");
+      }
+      return 0;
+    }
+  }
+
+  if (max_offset == curr_offset && buf[curr_offset] == '\0') {
+    max_offset++;
   }
 
   std::string::iterator it = name.end()-1;
   if( *it == '.') name.erase(it);
 
-  return ret;
+  return max_offset - offset;
 }
 
 size_t DNS::A_Record::parseFromBuffer(const u8 *buf, size_t offset, size_t maxlen)
diff --git a/nmap_dns.h b/nmap_dns.h
index 39c71c71e..b9de0f9a2 100644
--- a/nmap_dns.h
+++ b/nmap_dns.h
@@ -69,6 +69,8 @@ class Target;
 #include <algorithm>
 #include <sstream>
 
+#define DNS_LABEL_MAX_LENGTH 63
+#define DNS_NAME_MAX_LENGTH 255
 
 namespace DNS
 {