From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 11 Aug 2020 09:25:06 +0000 (+0200)
Subject: [PATCH 1/2] Raise an exception on invalid hex content in unknown records
X-Git-Tag: archive/raspbian/4.1.6-3+rpi1+deb10u1^2~1
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=7471ea21bb1331521ed89e3989834ab1cbe93ef1;p=pdns.git

[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 b6108d8..356e4a6 100644
--- 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 df4102a..770102f 100644
--- 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()