From ea9a8cb392ff59438a911485742b220d40f24d6f Mon Sep 17 00:00:00 2001
From: Evan Hunt <each@isc.org>
Date: Wed, 20 Dec 2023 00:32:57 -0800
Subject: [PATCH] prevent an infinite loop in fix_iterator()

it was possible for fix_iterator() to get stuck in a loop while
trying to find the predecessor of a missing node. this has been
fixed and a regression test has been added.
---
 lib/dns/qp.c        | 13 +++++++++++--
 tests/dns/qp_test.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/lib/dns/qp.c b/lib/dns/qp.c
index bf7e34b5c4..4ea05e0968 100644
--- a/lib/dns/qp.c
+++ b/lib/dns/qp.c
@@ -2115,6 +2115,17 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpnode_t *start,
 		return (leaf);
 	}
 
+	/*
+	 * Special case: if the search key differs even before the root
+	 * key offset, it means the name desired either precedes or
+	 * follows the entire range of names in the database, and
+	 * popping up the stack won't help us, so just move the
+	 * iterator one step back from the origin and return.
+	 */
+	if (to < branch_key_offset(iter->stack[0])) {
+		dns_qpiter_init(qp, iter);
+		return (prevleaf(iter));
+	}
 	/*
 	 * As long as the branch offset point is after the point where the
 	 * search key differs, we need to branch up and find a better leaf
@@ -2128,8 +2139,6 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpnode_t *start,
 			 * go to the parent branch and iterate back to the
 			 * predecessor from that point.
 			 */
-			iter->stack[iter->sp] = NULL;
-			iter->sp--;
 			n = prevleaf(iter);
 			leaf = n;
 		} else {
diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c
index 674a92cd4f..b9c3afcc5f 100644
--- a/tests/dns/qp_test.c
+++ b/tests/dns/qp_test.c
@@ -755,6 +755,48 @@ ISC_RUN_TEST_IMPL(predecessors) {
 	dns_qp_destroy(&qp);
 }
 
+/*
+ * this is a regression test for an infinite loop that could
+ * previously occur in fix_iterator()
+ */
+ISC_RUN_TEST_IMPL(fixiterator) {
+	dns_qp_t *qp = NULL;
+	const char insert[][32] = { "dynamic.",
+				    "a.dynamic.",
+				    "aaaa.dynamic.",
+				    "cdnskey.dynamic.",
+				    "cds.dynamic.",
+				    "cname.dynamic.",
+				    "dname.dynamic.",
+				    "dnskey.dynamic.",
+				    "ds.dynamic.",
+				    "mx.dynamic.",
+				    "ns.dynamic.",
+				    "nsec.dynamic.",
+				    "private-cdnskey.dynamic.",
+				    "private-dnskey.dynamic.",
+				    "rrsig.dynamic.",
+				    "txt.dynamic.",
+				    "" };
+	int i = 0;
+
+	dns_qp_create(mctx, &string_methods, NULL, &qp);
+	while (insert[i][0] != '\0') {
+		insert_str(qp, insert[i++]);
+	}
+
+	static struct check_predecessors check1[] = {
+		{ "newtext.dynamic.", "mx.dynamic.", DNS_R_PARTIALMATCH, 6 },
+		{ "absent.", "txt.dynamic.", ISC_R_NOTFOUND, 0 },
+		{ "nonexistent.", "txt.dynamic.", ISC_R_NOTFOUND, 0 },
+		{ NULL, NULL, 0, 0 }
+	};
+
+	check_predecessors(qp, check1);
+
+	dns_qp_destroy(&qp);
+}
+
 ISC_TEST_LIST_START
 ISC_TEST_ENTRY(qpkey_name)
 ISC_TEST_ENTRY(qpkey_sort)
@@ -762,6 +804,7 @@ ISC_TEST_ENTRY(qpiter)
 ISC_TEST_ENTRY(partialmatch)
 ISC_TEST_ENTRY(qpchain)
 ISC_TEST_ENTRY(predecessors)
+ISC_TEST_ENTRY(fixiterator)
 ISC_TEST_LIST_END
 
 ISC_TEST_MAIN