rec: fix the use of an uninitialized filtering policy

If `wantsRPZ` is set to false by the `prerpz` hook, `dfepol` might
not be correctly initialized. This leads to `appliedPolicy` not being
either before being passed to `preresolve` and `postresolve`.

Reported by Coverity.

diff --git a/pdns/filterpo.cc b/pdns/filterpo.cc
index 7e3f08266278d4aa1ca72004f53c286846469f8d..d32e8c150c7b80a6919901503f53604ec80a9c16 100644 (file)
--- a/pdns/filterpo.cc
+++ b/pdns/filterpo.cc
@@ -60,7 +60,7 @@ bool findNamedPolicy(const map<DNSName, DNSFilterEngine::Policy>& polmap, const
 DNSFilterEngine::Policy DNSFilterEngine::getProcessingPolicy(const DNSName& qname, const std::unordered_map<std::string,bool>& discardedPolicies) const
 {
   //  cout<<"Got question for nameserver name "<<qname<<endl;
-  Policy pol{PolicyKind::NoAction, nullptr, nullptr, 0};
+  Policy pol;
   for(const auto& z : d_zones) {
     if(z.name && discardedPolicies.find(*z.name) != discardedPolicies.end()) {
       continue;
@@ -87,13 +87,13 @@ DNSFilterEngine::Policy DNSFilterEngine::getProcessingPolicy(const ComboAddress&
       return fnd->second;;
     }
   }
-  return Policy{PolicyKind::NoAction, nullptr, nullptr, 0};
+  return Policy();
 }
 
 DNSFilterEngine::Policy DNSFilterEngine::getQueryPolicy(const DNSName& qname, const ComboAddress& ca, const std::unordered_map<std::string,bool>& discardedPolicies) const
 {
   //  cout<<"Got question for "<<qname<<" from "<<ca.toString()<<endl;
-  Policy pol{PolicyKind::NoAction, nullptr, nullptr, 0};
+  Policy pol;
   for(const auto& z : d_zones) {
     if(z.name && discardedPolicies.find(*z.name) != discardedPolicies.end()) {
       continue;
@@ -141,7 +141,7 @@ DNSFilterEngine::Policy DNSFilterEngine::getPostPolicy(const vector<DNSRecord>&
        return fnd->second;
     }
   }
-  return Policy{PolicyKind::NoAction, nullptr, nullptr, 0};
+  return Policy();
 }
 
 void DNSFilterEngine::assureZones(size_t zone)

diff --git a/pdns/filterpo.hh b/pdns/filterpo.hh
index b876f931e539dab8cd2364030c9553437654a6a8..283ea5bec71d1b9118876701b9cb671020f880bd 100644 (file)
--- a/pdns/filterpo.hh
+++ b/pdns/filterpo.hh
@@ -67,6 +67,9 @@ public:
   enum class PolicyKind { NoAction, Drop, NXDOMAIN, NODATA, Truncate, Custom};
   struct Policy
   {
+    Policy(): d_kind(PolicyKind::NoAction), d_custom(nullptr), d_name(nullptr), d_ttl(0)
+    {
+    }
     bool operator==(const Policy& rhs) const
     {
       return d_kind == rhs.d_kind; // XXX check d_custom too!

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index fa2b40c9c0db1d4554d1504da6593d6f667b4b9a..08bcf5b39ce505c44647121110671c3727a63d0c 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -656,7 +656,6 @@ void startDoResolve(void *p)
     vector<uint8_t> packet;
 
     auto luaconfsLocal = g_luaconfs.getLocal();
-    DNSFilterEngine::Policy appliedPolicy;
     // Used to tell syncres later on if we should apply NSDNAME and NSIP RPZ triggers for this query
     bool wantsRPZ(true);
     RecProtoBufMessage pbMessage(RecProtoBufMessage::Response);
@@ -711,7 +710,7 @@ void startDoResolve(void *p)
     bool shouldNotValidate = false;
 
     int res;
-    DNSFilterEngine::Policy dfepol;
+    DNSFilterEngine::Policy appliedPolicy;
     DNSRecord spoofed;
     if(dc->d_mdp.d_qtype==QType::ANY && !dc->d_tcp && g_anyToTcp) {
       pw.getHeader()->tc = 1;
@@ -747,9 +746,8 @@ void startDoResolve(void *p)
 
     // Check if the query has a policy attached to it
     if (wantsRPZ) {
-      dfepol = luaconfsLocal->dfe.getQueryPolicy(dc->d_mdp.d_qname, dc->d_remote, sr.d_discardedPolicies);
+      appliedPolicy = luaconfsLocal->dfe.getQueryPolicy(dc->d_mdp.d_qname, dc->d_remote, sr.d_discardedPolicies);
     }
-    appliedPolicy = dfepol;
 
     // if there is a RecursorLua active, and it 'took' the query in preResolve, we don't launch beginResolve
     if(!t_pdl->get() || !(*t_pdl)->preresolve(dc->d_remote, dc->d_local, dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), dc->d_tcp, ret, dc->d_ednsOpts.empty() ? 0 : &dc->d_ednsOpts, dc->d_tag, &appliedPolicy, &dc->d_policyTags, res, &variableAnswer, &wantsRPZ)) {
@@ -852,8 +850,7 @@ void startDoResolve(void *p)
       }
 
       if (wantsRPZ) {
-        dfepol = luaconfsLocal->dfe.getPostPolicy(ret, sr.d_discardedPolicies);
-        appliedPolicy = dfepol;
+        appliedPolicy = luaconfsLocal->dfe.getPostPolicy(ret, sr.d_discardedPolicies);
       }
 
       if(t_pdl->get()) {

diff --git a/pdns/rpzloader.cc b/pdns/rpzloader.cc
index 8e4b3cb9b2a50bc04074f096bfda7eb90c77dc14..1d2624d9eecf5be18e7ceadd23d05dc0d1dc7aea 100644 (file)
--- a/pdns/rpzloader.cc
+++ b/pdns/rpzloader.cc
@@ -64,7 +64,7 @@ void RPZRecordToPolicy(const DNSRecord& dr, DNSFilterEngine& target, bool addOrR
   static const DNSName rpzClientIP("rpz-client-ip"), rpzIP("rpz-ip"),
     rpzNSDname("rpz-nsdname"), rpzNSIP("rpz-nsip.");
 
-  DNSFilterEngine::Policy pol{DNSFilterEngine::PolicyKind::NoAction, nullptr, nullptr, 0};
+  DNSFilterEngine::Policy pol;
 
   if(dr.d_class != QClass::IN) {
     return;

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 7d0ef3826de0eef437cdd75967f7ede64a6579b9..8820d359680ff424f60e1e31738893beec57d7a1 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -358,7 +358,7 @@ public:
   static unsigned int s_maxqperq;
   static unsigned int s_maxtotusec;
   std::unordered_map<std::string,bool> d_discardedPolicies;
-  DNSFilterEngine::Policy d_appliedPolicy{DNSFilterEngine::PolicyKind::NoAction, nullptr, nullptr, 0};
+  DNSFilterEngine::Policy d_appliedPolicy;
   unsigned int d_outqueries;
   unsigned int d_tcpoutqueries;
   unsigned int d_throttledqueries;