From 5280f0038ff22a7856e12bef1c8b2aba8caffdd2 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Tue, 11 Feb 2025 10:19:29 +0100 Subject: [PATCH 1/2] rec: check bounds of rcode stats counter index (safe right now) Safe right now as LWResult::d_rcode gets assigned from the 4 bit rcode in the header. But that might change one day. I'd rather make LWResult::d_rcode an uint8_t, but that causes a conflict with the OOB resolving code that does not make a difference between res and d_rcode. --- pdns/recursordist/lwres.hh | 2 +- pdns/recursordist/rec-tcounters.hh | 4 ++-- pdns/recursordist/syncres.cc | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pdns/recursordist/lwres.hh b/pdns/recursordist/lwres.hh index 58d43535e32c..3b2bc43a1d02 100644 --- a/pdns/recursordist/lwres.hh +++ b/pdns/recursordist/lwres.hh @@ -79,10 +79,10 @@ public: } vector d_records; + uint32_t d_usec{0}; int d_rcode{0}; bool d_validpacket{false}; bool d_aabit{false}, d_tcbit{false}; - uint32_t d_usec{0}; bool d_haveEDNS{false}; }; diff --git a/pdns/recursordist/rec-tcounters.hh b/pdns/recursordist/rec-tcounters.hh index 80f7e41353d2..feeba0d6ab4e 100644 --- a/pdns/recursordist/rec-tcounters.hh +++ b/pdns/recursordist/rec-tcounters.hh @@ -195,8 +195,8 @@ struct Counters } return *this; } - static const size_t numberoOfRCodes = 16; - std::array rcodeCounters; + static const size_t numberOfRCodes = 16; + std::array rcodeCounters; }; // An RCodes histogram RCodeCounters auth{}; diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index 3e4f5a941d65..fd16be300777 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -5552,7 +5552,9 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, } accountAuthLatency(lwr.d_usec, remoteIP.sin4.sin_family); - ++t_Counters.at(rec::RCode::auth).rcodeCounters.at(static_cast(lwr.d_rcode)); + if (lwr.d_rcode >= 0 && lwr.d_rcode < static_cast(rec::Counters::RCodeCounters::numberOfRCodes)) { + ++t_Counters.at(rec::RCode::auth).rcodeCounters.at(static_cast(lwr.d_rcode)); + } if (!dontThrottle) { dontThrottle = shouldNotThrottle(&nsName, &remoteIP); From 85302693c7bff37a19113708b38463575f6749dc Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Tue, 11 Feb 2025 14:18:25 +0100 Subject: [PATCH 2/2] Apply suggestion Co-authored-by: Remi Gacogne --- pdns/recursordist/syncres.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index fd16be300777..53ff27f98f22 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -5552,7 +5552,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, } accountAuthLatency(lwr.d_usec, remoteIP.sin4.sin_family); - if (lwr.d_rcode >= 0 && lwr.d_rcode < static_cast(rec::Counters::RCodeCounters::numberOfRCodes)) { + if (lwr.d_rcode >= 0 && lwr.d_rcode < static_cast(t_Counters.at(rec::RCode::auth).rcodeCounters.size())) { ++t_Counters.at(rec::RCode::auth).rcodeCounters.at(static_cast(lwr.d_rcode)); }