From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Fri, 13 Jun 2025 13:27:10 +0000 (+0200)
Subject: lib/cache pkt_renew(): don't keep parts of packet header
X-Git-Tag: v6.0.15~2^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fenvironments%2Fdocs-pkt-renew-306vyc%2Fdeployments%2F7216;p=thirdparty%2Fknot-resolver.git

lib/cache pkt_renew(): don't keep parts of packet header

No idea why it's been done in this weird way since forever.
---

diff --git a/NEWS b/NEWS
index a89970f1f..4ed34f9a5 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,12 @@
 Knot Resolver 6.0.15 (2025-06-dd)
 =================================
 
+Security
+--------
+- DoS: fix a rare segfault in `resolve` function (!1717)
+  Someone controlling the DNS traffic might be able
+  to trigger this crash intentionally and too often.
+
 Bugfixes
 --------
 - manager: prometheus metrics update (!1703, #917, !1712)
diff --git a/lib/cache/knot_pkt.c b/lib/cache/knot_pkt.c
index 4b28891e9..7fe975cc2 100644
--- a/lib/cache/knot_pkt.c
+++ b/lib/cache/knot_pkt.c
@@ -11,14 +11,9 @@
 
 int pkt_renew(knot_pkt_t *pkt, const knot_dname_t *name, uint16_t type)
 {
-	/* Clear the packet if needed. */
-	if (pkt->rrset_count != 0 || !knot_dname_is_equal(knot_pkt_qname(pkt), name)
-	    || knot_pkt_qtype(pkt) != type || knot_pkt_qclass(pkt) != KNOT_CLASS_IN) {
-		int ret = kr_pkt_recycle(pkt);
-		if (ret) return kr_error(ret);
-		ret = knot_pkt_put_question(pkt, name, KNOT_CLASS_IN, type);
-		if (ret) return kr_error(ret);
-	}
+	knot_pkt_clear(pkt);
+	int ret = knot_pkt_put_question(pkt, name, KNOT_CLASS_IN, type);
+	if (ret) return kr_error(ret);
 
 	pkt->parsed = pkt->size = KR_PKT_SIZE_NOWIRE;
 	knot_wire_set_qr(pkt->wire);
diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index 3cc641cd8..9a9a41a77 100644
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -1060,6 +1060,8 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
 	if (!query) {
 		return ctx->state;
 	}
+	if (pkt->size == KR_PKT_SIZE_NOWIRE)
+		goto skip_checks; // answers from cache are sane, surely
 	query->flags.PKT_IS_SANE = false;
 
 	WITH_VERBOSE(query) {
@@ -1111,7 +1113,7 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
 		}
 		return KR_STATE_CONSUME;
 	}
-
+skip_checks:
 	/* If exiting above here, there's no sense to put it into packet cache.
 	 * Having "extra bytes" at the end of DNS message is considered SANE here.
 	 * The most important part is to check for spoofing: is_paired_to_query() */