From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Thu, 29 Jun 2023 07:31:46 +0000 (+0200)
Subject: rec: fix qname length getting out-of-sync with qname-minimization iteration count
X-Git-Tag: rec-5.0.0-alpha1~139^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7b9450932da11f34a8a729b7b7e47202276fff5f;p=thirdparty%2Fpdns.git

rec: fix qname length getting out-of-sync with qname-minimization iteration count

Approach two: fall back to non-QM mode if loop detected
Fixes #12956
---

diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index 056825cf64..c36aebb07b 100644
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -1599,7 +1599,12 @@ static unsigned int qmStepLen(unsigned int labels, unsigned int qnamelen, unsign
   return targetlen;
 }
 
-int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context)
+static string resToString(int res)
+{
+  return res >= 0 ? RCode::to_s(res) : std::to_string(res);
+}
+
+int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context) // NOLINT(readability-function-cognitive-complexity)
 {
   auto prefix = getPrefix(depth);
   auto luaconfsLocal = g_luaconfs.getLocal();
@@ -1733,21 +1738,24 @@ int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector<DNSRecord
       if (child == qname) {
         LOG(prefix << qname << ": Step3 Going to do final resolve" << endl);
         res = doResolveNoQNameMinimization(qname, qtype, ret, depth, beenthere, context);
-        LOG(prefix << qname << ": Step3 Final resolve: " << (res >= 0 ? RCode::to_s(res) : std::to_string(res)) << "/" << ret.size() << endl);
+        LOG(prefix << qname << ": Step3 Final resolve: " << resToString(res) << "/" << ret.size() << endl);
         return res;
       }
 
-      // If we have seen this child during resolution already; just skip it. We tried to QM it already or otherwise broken.
-      bool skipStep4 = false;
+      // If we have seen this child during resolution already; we tried to QM it already or otherwise broken.
+      // fall back to no-QM
+      bool qmLoopDetected = false;
       for (const auto& visitedNS : beenthere) {
         if (visitedNS.qname == child) {
-          skipStep4 = true;
+          qmLoopDetected = true;
           break;
         }
       }
-      if (skipStep4) {
-        LOG(prefix << qname << ": Step4 Being skipped as visited this child name already" << endl);
-        continue;
+      if (qmLoopDetected) {
+        LOG(prefix << qname << ": Step4 loop detected as visited this child name already, fallback to no QM" << endl);
+        res = doResolveNoQNameMinimization(qname, qtype, ret, depth, beenthere, context);
+        LOG(prefix << qname << ": Step4 Final resolve: " << resToString(res) << "/" << ret.size() << endl);
+        return res;
       }
 
       // Step 4
@@ -1783,7 +1791,7 @@ int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector<DNSRecord
           }
         }
 
-        LOG(prefix << qname << ": Step5 End resolve: " << RCode::to_s(res) << "/" << ret.size() << endl);
+        LOG(prefix << qname << ": Step5 End resolve: " << resToString(res) << "/" << ret.size() << endl);
         return res;
       }
     }