From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Mon, 29 Aug 2016 09:52:00 +0000 (+0200)
Subject: rec: fix the use of an uninitialized filtering policy
X-Git-Tag: dnsdist-1.1.0-beta1~8^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F4376%2Fhead;p=thirdparty%2Fpdns.git

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 7e3f082662..d32e8c150c 100644
--- 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 b876f931e5..283ea5bec7 100644
--- 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 fa2b40c9c0..08bcf5b39c 100644
--- 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 8e4b3cb9b2..1d2624d9ee 100644
--- 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 7d0ef3826d..8820d35968 100644
--- 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;