Add a minimum offset parameter to DNSName

PacketReader does not copy the header part of the DNS message,
therefore DNSName needs to be aware of the minimum valid offset
in order not to follow a pointer to an offset < sizeof(dnsheader),
as other callers expect every non-negative offset to be valid.
Found with American Fuzzy Lop and Address Sanitizer.

diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc
index 55849c5a9177885b473c4c65c10805c747915d28..e8e2c853b108ee23ba245036397169fbf7a98a01 100644 (file)
--- a/pdns/dnsname.cc
+++ b/pdns/dnsname.cc
@@ -57,7 +57,7 @@ DNSName::DNSName(const char* p)
 }
 
 
-DNSName::DNSName(const char* pos, int len, int offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed)
+DNSName::DNSName(const char* pos, int len, int offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, uint16_t minOffset)
 {
   if (offset >= len)
     throw std::range_error("Trying to read past the end of the buffer ("+std::to_string(offset)+ " >= "+std::to_string(len)+")");
@@ -68,11 +68,11 @@ DNSName::DNSName(const char* pos, int len, int offset, bool uncompress, uint16_t
     }
   }
 
-  packetParser(pos, len, offset, uncompress, qtype, qclass, consumed);
+  packetParser(pos, len, offset, uncompress, qtype, qclass, consumed, 0, minOffset);
 }
 
 // this should be the __only__ dns name parser in PowerDNS.
-void DNSName::packetParser(const char* qpos, int len, int offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, int depth)
+void DNSName::packetParser(const char* qpos, int len, int offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, int depth, uint16_t minOffset)
 {
   const unsigned char* pos=(const unsigned char*)qpos;
   unsigned char labellen;
@@ -80,6 +80,8 @@ void DNSName::packetParser(const char* qpos, int len, int offset, bool uncompres
 
   if (offset >= len)
     throw std::range_error("Trying to read past the end of the buffer ("+std::to_string(offset)+ " >= "+std::to_string(len)+")");
+  if (offset < (int) minOffset)
+    throw std::range_error("Trying to read before the beginning of the buffer ("+std::to_string(offset)+ " < "+std::to_string(minOffset)+")");
 
   const unsigned char* end = pos + len;
   pos += offset;
@@ -92,9 +94,11 @@ void DNSName::packetParser(const char* qpos, int len, int offset, bool uncompres
       int newpos = (labellen << 8) + *(const unsigned char*)pos;
 
       if(newpos < offset) {
+        if(newpos < (int) minOffset)
+          throw std::range_error("Invalid label position during decompression ("+std::to_string(newpos)+ " < "+std::to_string(minOffset)+")");
         if (++depth > 100)
           throw std::range_error("Abort label decompression after 100 redirects");
-        packetParser((const char*)opos, len, newpos, true, 0, 0, 0, depth);
+        packetParser((const char*)opos, len, newpos, true, 0, 0, 0, depth, minOffset);
       } else
         throw std::range_error("Found a forward reference during label decompression");
       pos++;

diff --git a/pdns/dnsname.hh b/pdns/dnsname.hh
index a0662cb26a3aa10b64778209b5f147fdc8cb8510..f2311655abfa372a97894419c28b4f9ede49a3ac 100644 (file)
--- a/pdns/dnsname.hh
+++ b/pdns/dnsname.hh
@@ -45,7 +45,7 @@ public:
   DNSName()  {}          //!< Constructs an *empty* DNSName, NOT the root!
   explicit DNSName(const char* p);      //!< Constructs from a human formatted, escaped presentation
   explicit DNSName(const std::string& str) : DNSName(str.c_str()) {}; //!< Constructs from a human formatted, escaped presentation
-  DNSName(const char* p, int len, int offset, bool uncompress, uint16_t* qtype=0, uint16_t* qclass=0, unsigned int* consumed=0); //!< Construct from a DNS Packet, taking the first question if offset=12
+  DNSName(const char* p, int len, int offset, bool uncompress, uint16_t* qtype=0, uint16_t* qclass=0, unsigned int* consumed=0, uint16_t minOffset=0); //!< Construct from a DNS Packet, taking the first question if offset=12
   
   bool isPartOf(const DNSName& rhs) const;   //!< Are we part of the rhs name?
   bool operator==(const DNSName& rhs) const; //!< DNS-native comparison (case insensitive) - empty compares to empty
@@ -112,7 +112,7 @@ public:
 private:
   string_t d_storage;
 
-  void packetParser(const char* p, int len, int offset, bool uncompress, uint16_t* qtype=0, uint16_t* qclass=0, unsigned int* consumed=0, int depth=0);
+  void packetParser(const char* p, int len, int offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, int depth, uint16_t minOffset);
   static std::string escapeLabel(const std::string& orig);
   static std::string unescapeLabel(const std::string& orig);
 };

diff --git a/pdns/dnsparser.cc b/pdns/dnsparser.cc
index 4e28afdad6701ba63b9d68bfc7e68ea67b98a2b3..d7eecd2e8e5ecb2e7c0a3f86ac6d9ed657ed9382 100644 (file)
--- a/pdns/dnsparser.cc
+++ b/pdns/dnsparser.cc
@@ -399,7 +399,7 @@ DNSName PacketReader::getName()
 {
   unsigned int consumed;
   try {
-    DNSName dn((const char*) d_content.data() - 12, d_content.size() + 12, d_pos + sizeof(dnsheader), true /* uncompress */, 0 /* qtype */, 0 /* qclass */, &consumed);
+    DNSName dn((const char*) d_content.data() - 12, d_content.size() + 12, d_pos + sizeof(dnsheader), true /* uncompress */, 0 /* qtype */, 0 /* qclass */, &consumed, sizeof(dnsheader));
     
     // the -12 fakery is because we don't have the header in 'd_content', but we do need to get 
     // the internal offsets to work