fix(dnsparser): disallow out of order or multiple SVC Params

diff --git a/pdns/dnsparser.cc b/pdns/dnsparser.cc
index 3809de27ff6e4b71854bdb600be6de71bea95084..8f17d81fff2444f54f10722e6db55b4b0ed6a2be 100644 (file)
--- a/pdns/dnsparser.cc
+++ b/pdns/dnsparser.cc
@@ -23,6 +23,9 @@
 #include "dnswriter.hh"
 #include <boost/algorithm/string.hpp>
 #include <boost/format.hpp>
+#include <cstdint>
+#include <stdexcept>
+#include <string>
 
 #include "dns_random.hh"
 #include "namespaces.hh"
@@ -591,13 +594,22 @@ void PacketReader::xfrBlob(string& blob, int length)
 }
 
 void PacketReader::xfrSvcParamKeyVals(set<SvcParam> &kvs) {
+  int32_t lastKey{-1}; // Keep track of the last key, as ordering should be strict
+
   while (d_pos < (d_startrecordpos + d_recordlen)) {
     if (d_pos + 2 > (d_startrecordpos + d_recordlen)) {
       throw std::out_of_range("incomplete key");
     }
     uint16_t keyInt;
     xfr16BitInt(keyInt);
+
+    if (keyInt <= lastKey) {
+      throw std::out_of_range("Found SVCParamKey " + std::to_string(keyInt) + " after SVCParamKey " + std::to_string(lastKey));
+    }
+    lastKey = keyInt;
+
     auto key = static_cast<SvcParam::SvcParamKey>(keyInt);
+
     uint16_t len;
     xfr16BitInt(len);
 

diff --git a/pdns/test-dnsparser_cc.cc b/pdns/test-dnsparser_cc.cc
index bfcc66842c0fab6ec94d767cbe62ef31208ec118..18467908b8c531d4fb2afa7cfd498b8b3cee723e 100644 (file)
--- a/pdns/test-dnsparser_cc.cc
+++ b/pdns/test-dnsparser_cc.cc
@@ -11,6 +11,8 @@
 #include <boost/test/unit_test.hpp>
 
 #include "dnsparser.hh"
+#include "dnswriter.hh"
+#include "svc-records.hh"
 
 BOOST_AUTO_TEST_SUITE(test_dnsparser_cc)
 
@@ -616,4 +618,45 @@ BOOST_AUTO_TEST_CASE(test_clearDNSPacketUnsafeRecordTypes) {
 
 }
 
+BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_multiple) {
+  vector<uint8_t> fakePacket {
+    0x00, 0x01, // Key 1 (ALPN)
+    0x00, 0x03, // length 3
+    0x02,           // ALPN length 2
+    'h', '2',
+
+    // A (forbidden) second ALPN
+    0x00, 0x01, // Key 1 (ALPN)
+    0x00, 0x03, // length 3
+    0x02,           // ALPN length 2
+    'h', '3'
+  };
+
+  PacketReader pr(std::string_view(reinterpret_cast<const char*>(fakePacket.data()), fakePacket.size()), 0);
+
+  std::set<SvcParam> svcParams;
+
+  BOOST_CHECK_THROW(pr.xfrSvcParamKeyVals(svcParams), std::out_of_range);
+}
+
+BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_bad_ordering) {
+  vector<uint8_t> fakePacket {
+    0x00, 0x04, // Key 4 (IPv4 Hint)
+    0x04,           // length 4 (1 address)
+    192, 168,
+    0, 1,
+
+    0x00, 0x01,  // Key 1 (ALPN)
+    0x00, 0x03, // length 3
+    0x02,           // ALPN length 2
+    'h', '2',
+  };
+
+  PacketReader pr(std::string_view(reinterpret_cast<const char*>(fakePacket.data()), fakePacket.size()), 0);
+
+  std::set<SvcParam> svcParams;
+
+  BOOST_CHECK_THROW(pr.xfrSvcParamKeyVals(svcParams), std::out_of_range);
+}
+
 BOOST_AUTO_TEST_SUITE_END()