CVE-2016-5427: Resource Management Errors (CWE-399)
value: PowerDNS (aka pdns) Authoritative Server before 3.4.10 does not properly handle a . (dot) inside labels, which allows remote attackers to cause a denial of service (backend CPU consumption) via a crafted DNS query.
CWE description: Weaknesses in this category are related to improper management of system resources.
CVSS v3 base score: 7.5
Published: 2016-09-21T14:25Z
Repository: https://github.com/PowerDNS/pdns
Language: C++
Files changed: pdns/dnsparser.cc, pdns/dnsparser.hh, pdns/misc.cc
Commit date: 2016-07-01 15:30:20 +0200
Commit message: Reject qname's wirelength > 255, `chopOff()` handle dot inside labels

Diff: diff --git a/pdns/dnsparser.cc b/pdns/dnsparser.cc
index c844f863..72761015 100644
--- a/pdns/dnsparser.cc
+++ b/pdns/dnsparser.cc
@@ -400,12 +400,13 @@ uint8_t PacketReader::get8BitInt()
 }
 
 string PacketReader::getLabel(unsigned int recurs)
 {
   string ret;
+  size_t wirelength = 0;
   ret.reserve(40);
-  getLabelFromContent(d_content, d_pos, ret, recurs++);
+  getLabelFromContent(d_content, d_pos, ret, recurs++, wirelength);
   return ret;
 }
 
 static string txtEscape(const string &name)
 {
@@ -451,19 +452,23 @@ string PacketReader::getText(bool multi)
 
   return ret;
 }
 
 
-void PacketReader::getLabelFromContent(const vector<uint8_t>& content, uint16_t& frompos, string& ret, int recurs) 
+void PacketReader::getLabelFromContent(const vector<uint8_t>& content, uint16_t& frompos, string& ret, int recurs, size_t& wirelength)
 {
   if(recurs > 100) // the forward reference-check below should make this test 100% obsolete
     throw MOADNSException("Loop");
 
   int pos = frompos;
   // it is tempting to call reserve on ret, but it turns out it creates a malloc/free storm in the loop
   for(;;) {
     unsigned char labellen=content.at(frompos++);
+    wirelength++;
+    if (wirelength > 255) {
+      throw MOADNSException("Overly long DNS name ("+lexical_cast<string>(wirelength)+")");
+    }
 
     if(!labellen) {
       if(ret.empty())
               ret.append(1,'.');
       break;
@@ -472,17 +477,21 @@ void PacketReader::getLabelFromContent(const vector<uint8_t>& content, uint16_t&
       uint16_t offset=256*(labellen & ~0xc0) + (unsigned int)content.at(frompos++) - sizeof(dnsheader);
       //        cout<<"This is an offset, need to go to: "<<offset<<endl;
 
       if(offset >= pos)
         throw MOADNSException("forward reference during label decompression");
-      return getLabelFromContent(content, offset, ret, ++recurs);
+      /* the compression pointer does not count into the wire length */
+      return getLabelFromContent(content, offset, ret, ++recurs, --wirelength);
     }
     else if(labellen > 63) 
       throw MOADNSException("Overly long label during label decompression ("+lexical_cast<string>((unsigned int)labellen)+")");
     else {
+      if (wirelength + labellen > 255) {
+        throw MOADNSException("Overly long DNS name ("+lexical_cast<string>(wirelength)+")");
+      }
+      wirelength += labellen;
       // XXX FIXME THIS MIGHT BE VERY SLOW!
-
       for(string::size_type n = 0 ; n < labellen; ++n, frompos++) {
         if(content.at(frompos)=='.' || content.at(frompos)=='\\') {
           ret.append(1, '\\');
           ret.append(1, content[frompos]);
         }
diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh
index 5fda67f2..60aab9ff 100644
--- a/pdns/dnsparser.hh
+++ b/pdns/dnsparser.hh
@@ -131,11 +131,11 @@ public:
   void xfrBlobNoSpaces(string& blob, int len);
   void xfrBlob(string& blob, int length);
   void xfrHexBlob(string& blob, bool keepReading=false);
 
   static uint16_t get16BitInt(const vector<unsigned char>&content, uint16_t& pos);
-  static void getLabelFromContent(const vector<uint8_t>& content, uint16_t& frompos, string& ret, int recurs);
+  static void getLabelFromContent(const vector<uint8_t>& content, uint16_t& frompos, string& ret, int recurs, size_t& wirelength);
 
   void getDnsrecordheader(struct dnsrecordheader &ah);
   void copyRecord(vector<unsigned char>& dest, uint16_t len);
   void copyRecord(unsigned char* dest, uint16_t len);
 
diff --git a/pdns/misc.cc b/pdns/misc.cc
index 32d396bc..9ad0d767 100644
--- a/pdns/misc.cc
+++ b/pdns/misc.cc
@@ -148,42 +148,65 @@ bool stripDomainSuffix(string *qname, const string &domain)
 bool chopOff(string &domain) 
 {
   if(domain.empty())
     return false;
 
-  string::size_type fdot=domain.find('.');
-
-  if(fdot==string::npos) 
-    domain="";
-  else {
-    string::size_type remain = domain.length() - (fdot + 1);
-    char tmp[remain];
-    memcpy(tmp, domain.c_str()+fdot+1, remain);
-    domain.assign(tmp, remain); // don't dare to do this w/o tmp holder :-)
+  bool escaped = false;
+  const string::size_type domainLen = domain.length();
+  for (size_t fdot = 0; fdot < domainLen; fdot++)
+  {
+    if (domain[fdot] == '.' && !escaped) {
+      string::size_type remain = domainLen - (fdot + 1);
+      char tmp[remain];
+      memcpy(tmp, domain.c_str()+fdot+1, remain);
+      domain.assign(tmp, remain); // don't dare to do this w/o tmp holder :-)
+
+      return true;
+    }
+    else if (domain[fdot] == '\\' && !escaped) {
+      escaped = true;
+    }
+    else {
+      escaped = false;
+    }
   }
+
+  domain = "";
   return true;
 }
 
 /** Chops off the start of a domain, so goes from 'www.ds9a.nl.' to 'ds9a.nl.' to 'nl.' to '.' Return zero on the empty string */
 bool chopOffDotted(string &domain)
 {
   if(domain.empty() || (domain.size()==1 && domain[0]=='.'))
     return false;
 
-  string::size_type fdot=domain.find('.');
-  if(fdot == string::npos)
-    return false;
-
-  if(fdot==domain.size()-1) 
-    domain=".";
-  else  {
-    string::size_type remain = domain.length() - (fdot + 1);
-    char tmp[remain];
-    memcpy(tmp, domain.c_str()+fdot+1, remain);
-    domain.assign(tmp, remain);
+  bool escaped = false;
+  const string::size_type domainLen = domain.length();
+  for (size_t fdot = 0; fdot < domainLen; fdot++)
+  {
+    if (domain[fdot] == '.' && !escaped) {
+      if (fdot==domain.size()-1) {
+        domain=".";
+      }
+      else {
+        string::size_type remain = domainLen - (fdot + 1);
+        char tmp[remain];
+        memcpy(tmp, domain.c_str()+fdot+1, remain);
+        domain.assign(tmp, remain); // don't dare to do this w/o tmp holder :-)
+      }
+      return true;
+    }
+    else if (domain[fdot] == '\\' && !escaped) {
+      escaped = true;
+    }
+    else {
+      escaped = false;
+    }
   }
-  return true;
+
+  return false;
 }
 
 
 bool ciEqual(const string& a, const string& b)
 {

Vulnerable code:   getLabelFromContent(d_content, d_pos, ret, recurs++);
void PacketReader::getLabelFromContent(const vector<uint8_t>& content, uint16_t& frompos, string& ret, int recurs) 
      return getLabelFromContent(content, offset, ret, ++recurs);

  static void getLabelFromContent(const vector<uint8_t>& content, uint16_t& frompos, string& ret, int recurs);
  string::size_type fdot=domain.find('.');

  if(fdot==string::npos) 
    domain="";
  else {
    string::size_type remain = domain.length() - (fdot + 1);
    char tmp[remain];
    memcpy(tmp, domain.c_str()+fdot+1, remain);
    domain.assign(tmp, remain); // don't dare to do this w/o tmp holder :-)
  string::size_type fdot=domain.find('.');
  if(fdot == string::npos)
    return false;

  if(fdot==domain.size()-1) 
    domain=".";
  else  {
    string::size_type remain = domain.length() - (fdot + 1);
    char tmp[remain];
    memcpy(tmp, domain.c_str()+fdot+1, remain);
    domain.assign(tmp, remain);
  return true;
Fixed code:   size_t wirelength = 0;
  getLabelFromContent(d_content, d_pos, ret, recurs++, wirelength);
void PacketReader::getLabelFromContent(const vector<uint8_t>& content, uint16_t& frompos, string& ret, int recurs, size_t& wirelength)
    wirelength++;
    if (wirelength > 255) {
      throw MOADNSException("Overly long DNS name ("+lexical_cast<string>(wirelength)+")");
    }
      /* the compression pointer does not count into the wire length */
      return getLabelFromContent(content, offset, ret, ++recurs, --wirelength);
      if (wirelength + labellen > 255) {
        throw MOADNSException("Overly long DNS name ("+lexical_cast<string>(wirelength)+")");
      }
      wirelength += labellen;
  static void getLabelFromContent(const vector<uint8_t>& content, uint16_t& frompos, string& ret, int recurs, size_t& wirelength);
  bool escaped = false;
  const string::size_type domainLen = domain.length();
  for (size_t fdot = 0; fdot < domainLen; fdot++)
  {
    if (domain[fdot] == '.' && !escaped) {
      string::size_type remain = domainLen - (fdot + 1);
      char tmp[remain];
      memcpy(tmp, domain.c_str()+fdot+1, remain);
      domain.assign(tmp, remain); // don't dare to do this w/o tmp holder :-)

      return true;
    }
    else if (domain[fdot] == '\\' && !escaped) {
      escaped = true;
    }
    else {
      escaped = false;
    }

  domain = "";
  bool escaped = false;
  const string::size_type domainLen = domain.length();
  for (size_t fdot = 0; fdot < domainLen; fdot++)
  {
    if (domain[fdot] == '.' && !escaped) {
      if (fdot==domain.size()-1) {
        domain=".";
      }
      else {
        string::size_type remain = domainLen - (fdot + 1);
        char tmp[remain];
        memcpy(tmp, domain.c_str()+fdot+1, remain);
        domain.assign(tmp, remain); // don't dare to do this w/o tmp holder :-)
      }
      return true;
    }
    else if (domain[fdot] == '\\' && !escaped) {
      escaped = true;
    }
    else {
      escaped = false;
    }

  return false;