File 881b5b03a590198d03008e4200dd00cc537712f3.patch of Package pdns

Overview Repositories Revisions Requests Users Attributes Meta

File 881b5b03a590198d03008e4200dd00cc537712f3.patch of Package pdns (Revision d37926806c31dbbf26eed68e678cbf3e)

Currently displaying revision d37926806c31dbbf26eed68e678cbf3e , Show latest

From 881b5b03a590198d03008e4200dd00cc537712f3 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 1 Jul 2016 15:30:20 +0200
Subject: [PATCH] Reject qname's wirelength > 255, `chopOff()` handle dot
 inside labels

Security advisory:
   https://doc.powerdns.com/md/security/powerdns-advisory-2016-01/
CVE: CVE-2016-5426, CVE-2016-5427

---
 pdns/dnsparser.cc | 17 +++++++++++----
 pdns/dnsparser.hh |  2 +-
 pdns/misc.cc      | 65 +++++++++++++++++++++++++++++++++++++------------------
 3 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/pdns/dnsparser.cc b/pdns/dnsparser.cc
index c844f86..7276101 100644
--- a/pdns/dnsparser.cc
+++ b/pdns/dnsparser.cc
@@ -402,8 +402,9 @@ 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;
 }
 
@@ -453,7 +454,7 @@ string PacketReader::getText(bool multi)
 }
 
 
-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");
@@ -462,6 +463,10 @@ void PacketReader::getLabelFromContent(const vector<uint8_t>& content, uint16_t&
   // 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())
@@ -474,13 +479,17 @@ void PacketReader::getLabelFromContent(const vector<uint8_t>& content, uint16_t&
 
       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, '\\');
diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh
index 5fda67f..60aab9f 100644
--- a/pdns/dnsparser.hh
+++ b/pdns/dnsparser.hh
@@ -133,7 +133,7 @@ public:
   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);
diff --git a/pdns/misc.cc b/pdns/misc.cc
index 32d396b..9ad0d76 100644
--- a/pdns/misc.cc
+++ b/pdns/misc.cc
@@ -150,16 +150,27 @@ 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;
 }
 
@@ -169,19 +180,31 @@ 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;
 }

Places

File 881b5b03a590198d03008e4200dd00cc537712f3.patch of Package pdns (Revision d37926806c31dbbf26eed68e678cbf3e)

Places