[PATCH 1/2] Raise an exception on invalid hex content in unknown records

Otherwise we can end up reading uninitialised memory from the stack,
possibly leaking information.
This is only an issue if the content is read from an untrusted source
and can be passed back to an attacker.

Gbp-Pq: Name CVE-2020-17482.patch

diff --git a/pdns/dnsparser.cc b/pdns/dnsparser.cc
index b6108d8c6bc5d6b9b09f9968fc4b7dbc1777f0ea..356e4a6cef6dde40df28420f08c9b6001d80a420 100644 (file)
--- a/pdns/dnsparser.cc
+++ b/pdns/dnsparser.cc
@@ -40,17 +40,29 @@ public:
     // parse the input
     vector<string> parts;
     stringtok(parts, zone);
-    if(parts.size()!=3 && !(parts.size()==2 && equals(parts[1],"0")) )
-      throw MOADNSException("Unknown record was stored incorrectly, need 3 fields, got "+std::to_string(parts.size())+": "+zone );
-    const string& relevant=(parts.size() > 2) ? parts[2] : "";
-    unsigned int total=pdns_stou(parts[1]);
-    if(relevant.size() % 2 || relevant.size() / 2 != total)
+    // we need exactly 3 parts, except if the length field is set to 0 then we only need 2
+    if (parts.size() != 3 && !(parts.size() == 2 && equals(parts.at(1), "0"))) {
+      throw MOADNSException("Unknown record was stored incorrectly, need 3 fields, got " + std::to_string(parts.size()) + ": " + zone);
+    }
+
+    if (parts.at(0) != "\\#") {
+      throw MOADNSException("Unknown record was stored incorrectly, first part should be '\\#', got '" + parts.at(0) + "'");
+    }
+
+    const string& relevant = (parts.size() > 2) ? parts.at(2) : "";
+    unsigned int total = pdns_stou(parts.at(1));
+    if (relevant.size() % 2 || (relevant.size() / 2) != total) {
       throw MOADNSException((boost::format("invalid unknown record length: size not equal to length field (%d != 2 * %d)") % relevant.size() % total).str());
+    }
+
     string out;
-    out.reserve(total+1);
-    for(unsigned int n=0; n < total; ++n) {
+    out.reserve(total + 1);
+
+    for (unsigned int n = 0; n < total; ++n) {
       int c;
-      sscanf(relevant.c_str()+2*n, "%02x", &c);
+      if (sscanf(&relevant.at(2*n), "%02x", &c) != 1) {
+        throw MOADNSException("unable to read data at position " + std::to_string(2 * n) + " from unknown record of size " + std::to_string(relevant.size()));
+      }
       out.append(1, (char)c);
     }
 

diff --git a/pdns/test-dnsrecords_cc.cc b/pdns/test-dnsrecords_cc.cc
index df4102a4fb8f531ed02bca34ff731f61b46f991e..770102f1fd6bdf4cc6cdd4129d49b250814d84cf 100644 (file)
--- a/pdns/test-dnsrecords_cc.cc
+++ b/pdns/test-dnsrecords_cc.cc
@@ -327,4 +327,39 @@ BOOST_AUTO_TEST_CASE(test_opt_record_out) {
   BOOST_CHECK_EQUAL(makeHexDump(std::string(pak.begin(),pak.end())), makeHexDump(packet));
 }
 
+// special record test, because Unknown record types are the worst
+BOOST_AUTO_TEST_CASE(test_unknown_records_in) {
+
+  auto validUnknown = DNSRecordContent::mastermake(static_cast<QType::typeenum>(65534), QClass::IN, "\\# 1 42");
+
+  // we need at least two parts
+  BOOST_CHECK_THROW(auto notEnoughPartsUnknown = DNSRecordContent::mastermake(static_cast<QType::typeenum>(65534), QClass::IN, "\\#"), MOADNSException);
+
+  // two parts are OK when the RDATA size is 0, not OK otherwise
+  auto validEmptyUnknown = DNSRecordContent::mastermake(static_cast<QType::typeenum>(65534), QClass::IN, "\\# 0");
+  BOOST_CHECK_THROW(auto twoPartsNotZeroUnknown = DNSRecordContent::mastermake(static_cast<QType::typeenum>(65534), QClass::IN, "\\# 1"), MOADNSException);
+
+  // the first part has to be "\#"
+  BOOST_CHECK_THROW(auto invalidFirstPartUnknown = DNSRecordContent::mastermake(static_cast<QType::typeenum>(65534), QClass::IN, "\\$ 0"), MOADNSException);
+
+  // RDATA length is not even
+  BOOST_CHECK_THROW(auto unevenUnknown = DNSRecordContent::mastermake(static_cast<QType::typeenum>(65534), QClass::IN, "\\# 1 A"), MOADNSException);
+
+  // RDATA length is not equal to the expected size
+  BOOST_CHECK_THROW(auto wrongRDATASizeUnknown = DNSRecordContent::mastermake(static_cast<QType::typeenum>(65534), QClass::IN, "\\# 2 AA"), MOADNSException);
+
+  // RDATA is invalid (invalid hex value)
+  try {
+    auto invalidRDATAUnknown = DNSRecordContent::mastermake(static_cast<QType::typeenum>(65534), QClass::IN, "\\# 1 JJ");
+    // we should not reach that code
+    BOOST_CHECK(false);
+    // but if we do let's see what we got (likely what was left over on the stack)
+    BOOST_CHECK_EQUAL(invalidRDATAUnknown->getZoneRepresentation(), "\\# 1 jj");
+  }
+  catch (const MOADNSException& e)
+  {
+    // it's expected to end up there
+  }
+}
+
 BOOST_AUTO_TEST_SUITE_END()