From dbbe10c34473769a1e9e9b37dfe2adf0673b222a Mon Sep 17 00:00:00 2001
From: Peter van Dijk <peter.van.dijk@powerdns.com>
Date: Fri, 21 Aug 2020 11:13:47 +0200
Subject: [PATCH] statbag: prevent re-declaration in production coded

---
 pdns/receiver.cc   |  8 +++++++-
 pdns/statbag.cc    | 14 ++++++++++++--
 pdns/statbag.hh    |  3 +++
 pdns/testrunner.cc |  1 +
 4 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/pdns/receiver.cc b/pdns/receiver.cc
index 3be7c8c1e0..6caf96898d 100644
--- a/pdns/receiver.cc
+++ b/pdns/receiver.cc
@@ -636,7 +636,13 @@ int main(int argc, char **argv)
     exit(1);
   }
   
-  declareStats();
+  try {
+    declareStats();
+  }
+  catch(PDNSException &PE) {
+    g_log<<Logger::Error<<"Exiting because: "<<PE.reason<<endl;
+    exit(1);
+  }
   S.blacklist("special-memory-usage");
 
   DLOG(g_log<<Logger::Warning<<"Verbose logging in effect"<<endl);
diff --git a/pdns/statbag.cc b/pdns/statbag.cc
index 0876b27323..521530d6a2 100644
--- a/pdns/statbag.cc
+++ b/pdns/statbag.cc
@@ -38,6 +38,7 @@
 StatBag::StatBag()
 {
   d_doRings=false;
+  d_allowRedeclare=false;
 }
 
 void StatBag::exists(const string &key)
@@ -106,8 +107,13 @@ StatType StatBag::getStatType(const string &item)
 void StatBag::declare(const string &key, const string &descrip, StatType statType)
 {
   if(d_stats.count(key)) {
-    *d_stats[key] = 0;
-    return;
+    if (d_allowRedeclare) {
+      *d_stats[key] = 0;
+      return;
+    }
+    else {
+      throw PDNSException("Attempt to re-declare statbag '"+key+"'");
+    }
   }
 
   auto i=make_unique<AtomicCounter>(0);
@@ -118,6 +124,10 @@ void StatBag::declare(const string &key, const string &descrip, StatType statTyp
 
 void StatBag::declare(const string &key, const string &descrip, StatBag::func_t func, StatType statType)
 {
+  if(d_funcstats.count(key) && !d_allowRedeclare) {
+    throw PDNSException("Attempt to re-declare func statbag '"+key+"'");
+  }
+
   d_funcstats[key]=func;
   d_keyDescrips[key]=descrip;
   d_statTypes[key]=statType;
diff --git a/pdns/statbag.hh b/pdns/statbag.hh
index 1887b7833a..611712c223 100644
--- a/pdns/statbag.hh
+++ b/pdns/statbag.hh
@@ -80,6 +80,7 @@ class StatBag
   typedef map<string, func_t> funcstats_t;
   funcstats_t d_funcstats;
   bool d_doRings;
+
   std::set<string> d_blacklist;
 
   void registerRingStats(const string& name);
@@ -147,6 +148,8 @@ public:
   string getValueStr(const string &key); //!< read a value behind a key, and return it as a string
   string getValueStrZero(const string &key); //!< read a value behind a key, and return it as a string, and zero afterwards
   void blacklist(const string &str);
+
+  bool d_allowRedeclare; // only set this true during tests, never in production code
 };
 
 inline void StatBag::deposit(const string &key, int value)
diff --git a/pdns/testrunner.cc b/pdns/testrunner.cc
index ea38f33a8f..7db0337ae2 100644
--- a/pdns/testrunner.cc
+++ b/pdns/testrunner.cc
@@ -27,5 +27,6 @@ static bool init_unit_test() {
 // entry point:
 int main(int argc, char* argv[])
 {
+  S.d_allowRedeclare = true;
   return boost::unit_test::unit_test_main( &init_unit_test, argc, argv );
 }
-- 
2.47.2