Merge pull request #8052 from omoerbeek/backport-8028-to-rec-4.2.x

Backport 8028 to rec 4.2.x: limit compression pointers to 14 bits

pdns/Makefile.am

@@ -1312,6 +1312,7 @@ testrunner_SOURCES = \
 	test-dnsparser_cc.cc \
 	test-dnsparser_hh.cc \
 	test-dnsrecords_cc.cc \
+	test-dnswriter_cc.cc \
 	test-ipcrypt_cc.cc \
 	test-iputils_hh.cc \
 	test-ixfr_cc.cc \

pdns/dnswriter.cc

@@ -76,6 +76,7 @@ dnsheader* DNSPacketWriter::getHeader()
 
 void DNSPacketWriter::startRecord(const DNSName& name, uint16_t qtype, uint32_t ttl, uint16_t qclass, DNSResourceRecord::Place place, bool compress)
 {
+  d_compress = compress;
   commit();
   d_rollbackmarker=d_content.size();
 
@@ -200,6 +201,7 @@ void DNSPacketWriter::xfrUnquotedText(const string& text, bool lenField)
 
 
 static constexpr bool l_verbose=false;
+static constexpr uint16_t maxCompressionOffset=16384;
 uint16_t DNSPacketWriter::lookupName(const DNSName& name, uint16_t* matchLen)
 {
   // iterate over the written labels, see if we find a match
@@ -266,7 +268,9 @@ uint16_t DNSPacketWriter::lookupName(const DNSName& name, uint16_t* matchLen)
         }
         if(!c)
           break;
-        pvect.push_back(iter - d_content.cbegin());
+        auto offset = iter - d_content.cbegin();
+        if (offset >= maxCompressionOffset) break; // compression pointers cannot point here
+        pvect.push_back(offset);
         iter+=*iter+1;
       }
     }
@@ -325,14 +329,14 @@ void DNSPacketWriter::xfrName(const DNSName& name, bool compress, bool)
 
   uint16_t li=0;
   uint16_t matchlen=0;
-  if(compress && (li=lookupName(name, &matchlen))) {
+  if(d_compress && compress && (li=lookupName(name, &matchlen)) && li < maxCompressionOffset) {
     const auto& dns=name.getStorage(); 
     if(l_verbose)
       cout<<"Found a substring of "<<matchlen<<" bytes from the back, offset: "<<li<<", dnslen: "<<dns.size()<<endl;
     // found a substring, if www.powerdns.com matched powerdns.com, we get back matchlen = 13
 
     unsigned int pos=d_content.size();
-    if(pos < 16384 && matchlen != dns.size()) {
+    if(pos < maxCompressionOffset && matchlen != dns.size()) {
       if(l_verbose)
         cout<<"Inserting pos "<<pos<<" for "<<name<<" for compressed case"<<endl;
       d_namepositions.push_back(pos);
@@ -351,7 +355,7 @@ void DNSPacketWriter::xfrName(const DNSName& name, bool compress, bool)
     unsigned int pos=d_content.size();
     if(l_verbose)
       cout<<"Found nothing, we are at pos "<<pos<<", inserting whole name"<<endl;
-    if(pos < 16384) {
+    if(pos < maxCompressionOffset) {
       if(l_verbose)
         cout<<"Inserting pos "<<pos<<" for "<<name<<" for uncompressed case"<<endl;
       d_namepositions.push_back(pos);

pdns/dnswriter.hh

@@ -167,7 +167,7 @@ private:
 
   uint16_t d_truncatemarker; // end of header, for truncate
   DNSResourceRecord::Place d_recordplace;
-  bool d_canonic, d_lowerCase;
+  bool d_canonic, d_lowerCase, d_compress{false};
 };
 
 typedef vector<pair<string::size_type, string::size_type> > labelparts_t;

pdns/test-dnswriter_cc.cc

@@ -0,0 +1,81 @@
+#define BOOST_TEST_DYN_LINK
+#define BOOST_TEST_NO_MAIN
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <boost/test/unit_test.hpp>
+#include <fstream>
+
+#include "dnswriter.hh"
+#include "dnsparser.hh"
+
+BOOST_AUTO_TEST_SUITE(test_dnswriter_cc)
+
+BOOST_AUTO_TEST_CASE(test_compressionBool) {
+  auto testCompressionBool = [](bool compress, size_t size1, size_t size2) {
+    DNSName name("powerdns.com.");
+
+    vector<uint8_t> packet;
+    DNSPacketWriter pwR(packet, name, QType::A, QClass::IN, 0);
+    pwR.getHeader()->qr = 1;
+
+    pwR.startRecord(DNSName("mediumsizedlabel.example.net"), QType::A, 3600, QClass::IN, DNSResourceRecord::ANSWER, compress);
+    pwR.xfrIP('P'<<24 |
+              'Q'<<16 |
+              'R'<<8  |
+              'S');
+    pwR.commit();
+    BOOST_CHECK_EQUAL(pwR.size(), size1);
+
+    pwR.startRecord(DNSName("adifferentlabel.example.net"), QType::NS, 3600, QClass::IN, DNSResourceRecord::ANSWER, compress);
+    pwR.xfrName(DNSName("target.example.net"), true);
+    pwR.commit();
+    BOOST_CHECK_EQUAL(pwR.size(), size2);
+
+    string spacket(packet.begin(), packet.end());
+
+    BOOST_CHECK_NO_THROW(MOADNSParser mdp(false, spacket));
+  };
+
+  testCompressionBool(true, 74, 111);
+  testCompressionBool(false, 74, 133);
+}
+
+BOOST_AUTO_TEST_CASE(test_compressionBoundary) {
+  DNSName name("powerdns.com.");
+
+  vector<uint8_t> packet;
+  DNSPacketWriter pwR(packet, name, QType::A, QClass::IN, 0);
+  pwR.getHeader()->qr = 1;
+
+  /* record we want to see altered */
+  pwR.startRecord(name, QType::TXT, 3600, QClass::IN, DNSResourceRecord::ANSWER);
+  auto txt = string("\"")+string(16262, 'A')+string("\"");
+  pwR.xfrText(txt);
+  pwR.commit();
+  BOOST_CHECK_EQUAL(pwR.size(), 16368);
+
+  pwR.startRecord(DNSName("mediumsizedlabel.example.net"), QType::A, 3600, QClass::IN, DNSResourceRecord::ANSWER);
+  pwR.xfrIP('P'<<24 |
+            'Q'<<16 |
+            'R'<<8  |
+            'S');
+  pwR.commit();
+  BOOST_CHECK_EQUAL(pwR.size(), 16412); // 16412 (0x401c) puts '7example3net' at 0x4001
+
+  pwR.startRecord(DNSName("adifferentlabel.example.net"), QType::A, 3600, QClass::IN, DNSResourceRecord::ANSWER);
+  pwR.xfrIP('D'<<24 |
+            'E'<<16 |
+            'F'<<8  |
+            'G');
+  pwR.commit();
+  BOOST_CHECK_EQUAL(pwR.size(), 16455);
+
+  string spacket(packet.begin(), packet.end());
+
+  BOOST_CHECK_NO_THROW(MOADNSParser mdp(false, spacket));
+}
+
+BOOST_AUTO_TEST_SUITE_END()