dnsdist: Get rid of assert()

PowerDNS Security Advisory 2024-03 has made it clear that some of
them that have been designed to break during testing might break in
production, as we compile with `NDEBUG` unset.

diff --git a/pdns/dnsdistdist/dnsdist-ecs.cc b/pdns/dnsdistdist/dnsdist-ecs.cc
index 78ab5e14b1845cf0a443f9b2320aa4cc78f32bf7..988345640745df06b77b93ea0912671543d73d61 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-ecs.cc
+++ b/pdns/dnsdistdist/dnsdist-ecs.cc
@@ -44,7 +44,10 @@ bool g_addEDNSToSelfGeneratedResponses{true};
 
 int rewriteResponseWithoutEDNS(const PacketBuffer& initialPacket, PacketBuffer& newContent)
 {
-  assert(initialPacket.size() >= sizeof(dnsheader));
+  if (initialPacket.size() < sizeof(dnsheader)) {
+    return ENOENT;
+  }
+
   const dnsheader_aligned dnsHeader(initialPacket.data());
 
   if (ntohs(dnsHeader->arcount) == 0) {
@@ -153,7 +156,10 @@ static bool addOrReplaceEDNSOption(std::vector<std::pair<uint16_t, std::string>>
 
 bool slowRewriteEDNSOptionInQueryWithRecords(const PacketBuffer& initialPacket, PacketBuffer& newContent, bool& ednsAdded, uint16_t optionToReplace, bool& optionAdded, bool overrideExisting, const string& newOptionContent)
 {
-  assert(initialPacket.size() >= sizeof(dnsheader));
+  if (initialPacket.size() < sizeof(dnsheader)) {
+    return false;
+  }
+
   const dnsheader_aligned dnsHeader(initialPacket.data());
 
   if (ntohs(dnsHeader->qdcount) == 0) {
@@ -321,9 +327,10 @@ static bool slowParseEDNSOptions(const PacketBuffer& packet, EDNSOptionViewMap&
 
 int locateEDNSOptRR(const PacketBuffer& packet, uint16_t* optStart, size_t* optLen, bool* last)
 {
-  assert(optStart != nullptr);
-  assert(optLen != nullptr);
-  assert(last != nullptr);
+  if (optStart == nullptr || optLen == nullptr || last == nullptr) {
+    throw std::runtime_error("Invalid values passed to locateEDNSOptRR");
+  }
+
   const dnsheader_aligned dnsHeader(packet.data());
 
   if (ntohs(dnsHeader->arcount) == 0) {
@@ -390,8 +397,10 @@ int locateEDNSOptRR(const PacketBuffer& packet, uint16_t* optStart, size_t* optL
 /* extract the start of the OPT RR in a QUERY packet if any */
 int getEDNSOptionsStart(const PacketBuffer& packet, const size_t offset, uint16_t* optRDPosition, size_t* remaining)
 {
-  assert(optRDPosition != nullptr);
-  assert(remaining != nullptr);
+  if (optRDPosition == nullptr || remaining == nullptr) {
+    throw std::runtime_error("Invalid values passed to getEDNSOptionsStart");
+  }
+
   const dnsheader_aligned dnsHeader(packet.data());
 
   if (offset >= packet.size()) {
@@ -474,8 +483,9 @@ bool generateOptRR(const std::string& optRData, PacketBuffer& res, size_t maximu
 
 static bool replaceEDNSClientSubnetOption(PacketBuffer& packet, size_t maximumSize, size_t const oldEcsOptionStartPosition, size_t const oldEcsOptionSize, size_t const optRDLenPosition, const string& newECSOption)
 {
-  assert(oldEcsOptionStartPosition < packet.size());
-  assert(optRDLenPosition < packet.size());
+  if (oldEcsOptionStartPosition >= packet.size() || optRDLenPosition >= packet.size()) {
+    throw std::runtime_error("Invalid values passed to replaceEDNSClientSubnetOption");
+  }
 
   if (newECSOption.size() == oldEcsOptionSize) {
     /* same size as the existing option */
@@ -593,7 +603,9 @@ static bool addEDNSWithECS(PacketBuffer& packet, size_t maximumSize, const strin
 
 bool handleEDNSClientSubnet(PacketBuffer& packet, const size_t maximumSize, const size_t qnameWireLength, bool& ednsAdded, bool& ecsAdded, bool overrideExisting, const string& newECSOption)
 {
-  assert(qnameWireLength <= packet.size());
+  if (qnameWireLength > packet.size()) {
+    throw std::runtime_error("Invalid value passed to handleEDNSClientSubnet");
+  }
 
   const dnsheader_aligned dnsHeader(packet.data());
 
@@ -763,7 +775,10 @@ bool isEDNSOptionInOpt(const PacketBuffer& packet, const size_t optStart, const
 
 int rewriteResponseWithoutEDNSOption(const PacketBuffer& initialPacket, const uint16_t optionCodeToSkip, PacketBuffer& newContent)
 {
-  assert(initialPacket.size() >= sizeof(dnsheader));
+  if (initialPacket.size() < sizeof(dnsheader)) {
+    return ENOENT;
+  }
+
   const dnsheader_aligned dnsHeader(initialPacket.data());
 
   if (ntohs(dnsHeader->arcount) == 0) {

diff --git a/pdns/ednsoptions.cc b/pdns/ednsoptions.cc
index 8221f7201907e3c2f2ab24410d50d798c981b802..93c127e554c5992f11647097266d22d0e6a43963 100644 (file)
--- a/pdns/ednsoptions.cc
+++ b/pdns/ednsoptions.cc
@@ -45,12 +45,14 @@ bool getNextEDNSOption(const char* data, size_t dataLen, uint16_t& optionCode, u
 /* extract the position (relative to the optRR pointer!) and size of a specific EDNS0 option from a pointer on the beginning rdLen of the OPT RR */
 int getEDNSOption(const char* optRR, const size_t len, uint16_t wantedOption, size_t* optionValuePosition, size_t * optionValueSize)
 {
-  assert(optRR != nullptr);
-  assert(optionValuePosition != nullptr);
-  assert(optionValueSize != nullptr);
+  if (optRR == nullptr || optionValuePosition == nullptr || optionValueSize == nullptr) {
+    return EINVAL;
+  }
+
   size_t pos = 0;
-  if (len < DNS_RDLENGTH_SIZE)
+  if (len < DNS_RDLENGTH_SIZE) {
     return EINVAL;
+  }
 
   const uint16_t rdLen = (((unsigned char) optRR[pos]) * 256) + ((unsigned char) optRR[pos+1]);
   size_t rdPos = 0;
@@ -94,10 +96,10 @@ int getEDNSOption(const char* optRR, const size_t len, uint16_t wantedOption, si
 /* extract all EDNS0 options from a pointer on the beginning rdLen of the OPT RR */
 int getEDNSOptions(const char* optRR, const size_t len, EDNSOptionViewMap& options)
 {
-  assert(optRR != nullptr);
   size_t pos = 0;
-  if (len < DNS_RDLENGTH_SIZE)
+  if (optRR == nullptr || len < DNS_RDLENGTH_SIZE) {
     return EINVAL;
+  }
 
   const uint16_t rdLen = (((unsigned char) optRR[pos]) * 256) + ((unsigned char) optRR[pos+1]);
   size_t rdPos = 0;

diff --git a/pdns/ipcipher.cc b/pdns/ipcipher.cc
index 1d64cc902dd759d6a4f136daba8bb9d5676b68f4..d4782aafa65949744511a0e19910325f53c255c7 100644 (file)
--- a/pdns/ipcipher.cc
+++ b/pdns/ipcipher.cc
@@ -77,7 +77,9 @@ static ComboAddress encryptCA6(const ComboAddress& address, const std::string& k
   const auto inSize = sizeof(address.sin6.sin6_addr.s6_addr);
   static_assert(inSize == 16, "We disable padding and so we must assume a data size of 16 bytes");
   const auto blockSize = EVP_CIPHER_get_block_size(aes128cbc.get());
-  assert(blockSize == 16);
+  if (blockSize != 16) {
+    throw pdns::OpenSSL::error("encryptCA6: unexpected block size");
+  }
   EVP_CIPHER_CTX_set_padding(ctx.get(), 0);
 
   int updateLen = 0;
@@ -94,7 +96,9 @@ static ComboAddress encryptCA6(const ComboAddress& address, const std::string& k
     throw pdns::OpenSSL::error("encryptCA6: Could not finalize address encryption");
   }
 
-  assert(updateLen + finalLen == inSize);
+  if ((updateLen + finalLen) != inSize) {
+    throw pdns::OpenSSL::error("encryptCA6: unexpected final size");
+  }
 #else
   AES_KEY wctx;
   AES_set_encrypt_key((const unsigned char*)key.c_str(), 128, &wctx);
@@ -130,7 +134,9 @@ static ComboAddress decryptCA6(const ComboAddress& address, const std::string& k
   const auto inSize = sizeof(address.sin6.sin6_addr.s6_addr);
   static_assert(inSize == 16, "We disable padding and so we must assume a data size of 16 bytes");
   const auto blockSize = EVP_CIPHER_get_block_size(aes128cbc.get());
-  assert(blockSize == 16);
+  if (blockSize != 16) {
+    throw pdns::OpenSSL::error("decryptCA6: unexpected block size");
+  }
   EVP_CIPHER_CTX_set_padding(ctx.get(), 0);
 
   int updateLen = 0;
@@ -147,7 +153,9 @@ static ComboAddress decryptCA6(const ComboAddress& address, const std::string& k
     throw pdns::OpenSSL::error("decryptCA6: Could not finalize address decryption");
   }
 
-  assert(updateLen + finalLen == inSize);
+  if ((updateLen + finalLen) != inSize) {
+    throw pdns::OpenSSL::error("decryptCA6: unexpected final size");
+  }
 #else
   AES_KEY wctx;
   AES_set_decrypt_key((const unsigned char*)key.c_str(), 128, &wctx);