dnsdist: Properly handle invalid regular expressions

Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>

diff --git a/pdns/dnsdistdist/dnsdist-rules-factory.hh b/pdns/dnsdistdist/dnsdist-rules-factory.hh
index 36bc0519f452e3bad20b39507e2d9e0898143694..81cf10b99500f967b306b333d49cc8384188146b 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-rules-factory.hh
+++ b/pdns/dnsdistdist/dnsdist-rules-factory.hh
@@ -501,12 +501,18 @@ class RegexRule : public DNSRule
 {
 public:
   RegexRule(const std::string& regex) :
-    d_regex(regex), d_visual(regex)
+    d_visual(regex)
   {
+    try {
+      d_regex = Regex(regex);
+    }
+    catch (const PDNSException& exp) {
+      throw std::runtime_error("Error compiling expression in RegexRule: " + exp.reason);
+    }
   }
   bool matches(const DNSQuestion* dq) const override
   {
-    return d_regex.match(dq->ids.qname.toStringNoDot());
+    return d_regex->match(dq->ids.qname.toStringNoDot());
   }
 
   string toString() const override
@@ -515,7 +521,7 @@ public:
   }
 
 private:
-  Regex d_regex;
+  std::optional<Regex> d_regex{std::nullopt};
   string d_visual;
 };
 
@@ -571,7 +577,7 @@ public:
 
 private:
   string d_header;
-  Regex d_regex;
+  std::optional<Regex> d_regex{std::nullopt};
   string d_visual;
 };
 
@@ -594,7 +600,7 @@ public:
   string toString() const override;
 
 private:
-  Regex d_regex;
+  std::optional<Regex> d_regex{std::nullopt};
   std::string d_visual;
 };
 

diff --git a/pdns/dnsdistdist/dnsdist-rules.cc b/pdns/dnsdistdist/dnsdist-rules.cc
index e13cdbd5fccd5cab2e4701b4d165b7e72f404670..97b31bf9b965be1d7e6163f7958ef21e8bab6f5b 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-rules.cc
+++ b/pdns/dnsdistdist/dnsdist-rules.cc
@@ -25,10 +25,17 @@ std::atomic<uint64_t> LuaFFIPerThreadRule::s_functionsCounter = 0;
 thread_local std::map<uint64_t, LuaFFIPerThreadRule::PerThreadState> LuaFFIPerThreadRule::t_perThreadStates;
 
 HTTPHeaderRule::HTTPHeaderRule(const std::string& header, const std::string& regex) :
-  d_header(toLower(header)), d_regex(regex), d_visual("http[" + header + "] ~ " + regex)
+  d_header(toLower(header)), d_visual("http[" + header + "] ~ " + regex)
 {
 #if !defined(HAVE_DNS_OVER_HTTPS) && !defined(HAVE_DNS_OVER_HTTP3)
   throw std::runtime_error("Using HTTPHeaderRule while DoH support is not enabled");
+#else
+  try {
+    d_regex = Regex(regex);
+  }
+  catch (const PDNSException& exp) {
+    throw std::runtime_error("Error compiling expression in HTTPHeaderRule: " + exp.reason);
+  }
 #endif /* HAVE_DNS_OVER_HTTPS || HAVE_DNS_OVER_HTTP3 */
 }
 
@@ -39,7 +46,7 @@ bool HTTPHeaderRule::matches([[maybe_unused]] const DNSQuestion* dnsQuestion) co
     const auto& headers = dnsQuestion->ids.du->getHTTPHeaders();
     for (const auto& header : headers) {
       if (header.first == d_header) {
-        return d_regex.match(header.second);
+        return d_regex->match(header.second);
       }
     }
     return false;
@@ -50,7 +57,7 @@ bool HTTPHeaderRule::matches([[maybe_unused]] const DNSQuestion* dnsQuestion) co
     const auto& headers = dnsQuestion->ids.doh3u->getHTTPHeaders();
     for (const auto& header : headers) {
       if (header.first == d_header) {
-        return d_regex.match(header.second);
+        return d_regex->match(header.second);
       }
     }
     return false;
@@ -94,10 +101,17 @@ string HTTPPathRule::toString() const
 }
 
 HTTPPathRegexRule::HTTPPathRegexRule(const std::string& regex) :
-  d_regex(regex), d_visual("http path ~ " + regex)
+  d_visual("http path ~ " + regex)
 {
 #if !defined(HAVE_DNS_OVER_HTTPS) && !defined(HAVE_DNS_OVER_HTTP3)
   throw std::runtime_error("Using HTTPRegexRule while DoH support is not enabled");
+#else
+  try {
+    d_regex = Regex(regex);
+  }
+  catch (const PDNSException& exp) {
+    throw std::runtime_error("Error compiling expression in HTTPPathRegexRule: " + exp.reason);
+  }
 #endif /* HAVE_DNS_OVER_HTTPS || HAVE_DNS_OVER_HTTP3 */
 }
 
@@ -106,12 +120,12 @@ bool HTTPPathRegexRule::matches([[maybe_unused]] const DNSQuestion* dnsQuestion)
 #if defined(HAVE_DNS_OVER_HTTPS)
   if (dnsQuestion->ids.du) {
     const auto path = dnsQuestion->ids.du->getHTTPPath();
-    return d_regex.match(path);
+    return d_regex->match(path);
   }
 #endif /* HAVE_DNS_OVER_HTTPS */
 #if defined(HAVE_DNS_OVER_HTTP3)
   if (dnsQuestion->ids.doh3u) {
-    return d_regex.match(dnsQuestion->ids.doh3u->getHTTPPath());
+    return d_regex->match(dnsQuestion->ids.doh3u->getHTTPPath());
   }
   return false;
 #endif /* HAVE_DNS_OVER_HTTP3 */

diff --git a/pdns/misc.cc b/pdns/misc.cc
index 4f1e5c482b9ce7f63813751e64b77ddfb12cedb5..92921d370769459b7a740602409e1a4faacd5647 100644 (file)
--- a/pdns/misc.cc
+++ b/pdns/misc.cc
@@ -825,10 +825,15 @@ bool readFileIfThere(const char* fname, std::string* line)
   return stringfgets(filePtr.get(), *line);
 }
 
-Regex::Regex(const string &expr)
+Regex::Regex(const string& expr)
 {
-  if(regcomp(&d_preg, expr.c_str(), REG_ICASE|REG_NOSUB|REG_EXTENDED))
-    throw PDNSException("Regular expression did not compile");
+  if (auto ret = regcomp(&d_preg, expr.c_str(), REG_ICASE|REG_NOSUB|REG_EXTENDED); ret != 0) {
+    std::array<char, 1024> errorBuffer{};
+    if (regerror(ret, &d_preg, errorBuffer.data(), errorBuffer.size()) > 0) {
+      throw PDNSException("Regular expression " + expr + " did not compile: " + errorBuffer.data());
+    }
+    throw PDNSException("Regular expression " + expr + " did not compile");
+  }
 }
 
 // if you end up here because valgrind told you were are doing something wrong