From: Markus Koschany <apo@debian.org>
Date: Mon, 11 Mar 2024 14:24:02 +0100
Subject: CVE-2019-10191

Bug-Debian: https://bugs.debian.org/932048
Origin: https://gitlab.labs.nic.cz/knot/knot-resolver/merge_requests/839
---
 daemon/lua/kres-gen.lua |  1 +
 daemon/worker.c         |  5 ++++-
 lib/cache/api.c         |  4 +++-
 lib/cache/impl.h        |  3 ++-
 lib/layer.h             |  7 ++++++-
 lib/layer/iterate.c     | 11 ++++++++++-
 lib/resolve.c           |  2 ++
 lib/rplan.h             |  2 ++
 8 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/daemon/lua/kres-gen.lua b/daemon/lua/kres-gen.lua
index eeb8ff7..9e9f586 100644
--- a/daemon/lua/kres-gen.lua
+++ b/daemon/lua/kres-gen.lua
@@ -120,6 +120,7 @@ struct kr_qflags {
 	_Bool DNS64_MARK : 1;
 	_Bool CACHE_TRIED : 1;
 	_Bool NO_NS_FOUND : 1;
+	_Bool PKT_IS_SANE : 1;
 };
 typedef struct {
 	knot_rrset_t **at;
diff --git a/daemon/worker.c b/daemon/worker.c
index 117cc91..1235979 100644
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -1611,8 +1611,11 @@ int worker_submit(struct session *session, knot_pkt_t *query)
 			return kr_error(ENOMEM);
 		}
 	} else if (query) { /* response from upstream */
-		task = session_tasklist_del_msgid(session, knot_wire_get_id(query->wire));
+		const uint16_t id = knot_wire_get_id(query->wire);
+		task = session_tasklist_del_msgid(session, id);
 		if (task == NULL) {
+			VERBOSE_MSG(NULL, "=> ignoring packet with mismatching ID %d\n",
+					(int)id);
 			return kr_error(ENOENT);
 		}
 		assert(!session_flags(session)->closing);
diff --git a/lib/cache/api.c b/lib/cache/api.c
index c0591d6..4c7f3d2 100644
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -408,7 +408,9 @@ int cache_stash(kr_layer_t *ctx, knot_pkt_t *pkt)
 	/* LATER(optim.): typically we also have corresponding NS record in the list,
 	 * so we might save a cache operation. */
 
-	stash_pkt(pkt, qry, req, has_optout);
+	if (qry->flags.PKT_IS_SANE && check_dname_for_lf(knot_pkt_qname(pkt), qry)) {
+		stash_pkt(pkt, qry, req, has_optout);
+	}
 
 finally:
 	if (unauth_cnt) {
diff --git a/lib/cache/impl.h b/lib/cache/impl.h
index a08f355..ffb5e67 100644
--- a/lib/cache/impl.h
+++ b/lib/cache/impl.h
@@ -253,7 +253,8 @@ void entry_list_memcpy(struct entry_apex *ea, entry_list_t list);
 /* Packet caching; implementation in ./entry_pkt.c */
 
 /** Stash the packet into cache (if suitable, etc.)
- * \param has_optout whether the packet contains an opt-out NSEC3 */
+ * \param has_optout whether the packet contains an opt-out NSEC3
+ * It assumes check_dname_for_lf(). */
 void stash_pkt(const knot_pkt_t *pkt, const struct kr_query *qry,
 		const struct kr_request *req, bool has_optout);
 
diff --git a/lib/layer.h b/lib/layer.h
index 0909cb7..011d279 100644
--- a/lib/layer.h
+++ b/lib/layer.h
@@ -48,7 +48,12 @@
 enum kr_layer_state {
 	KR_STATE_CONSUME = 1 << 0, /*!< Consume data. */
 	KR_STATE_PRODUCE = 1 << 1, /*!< Produce data. */
-	KR_STATE_DONE    = 1 << 2, /*!< Finished successfully. */
+
+	/*! Finished successfully or a special case: in CONSUME phase this can
+	 * be used (by iterator) to do a transition to PRODUCE phase again,
+	 * in which case the packet wasn't accepted for some reason. */
+	KR_STATE_DONE    = 1 << 2,
+
 	KR_STATE_FAIL    = 1 << 3, /*!< Error. */
 	KR_STATE_YIELD   = 1 << 4, /*!< Paused, waiting for a sub-query. */
 };
diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index ee2607e..feaadf0 100644
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -81,10 +81,14 @@ static bool is_paired_to_query(const knot_pkt_t *answer, struct kr_query *query)
 	uint16_t qtype = query->stype;
 	const knot_dname_t *qname = minimized_qname(query, &qtype);
 
+	/* ID should already match, thanks to session_tasklist_del_msgid()
+	 * in worker_submit(), but it won't hurt to check again. */
 	return query->id      == knot_wire_get_id(answer->wire) &&
-	       knot_wire_get_qdcount(answer->wire) > 0 &&
+	       knot_wire_get_qdcount(answer->wire) == 1 &&
 	       query->sclass  == knot_pkt_qclass(answer) &&
 	       qtype          == knot_pkt_qtype(answer) &&
+	       /* qry->secret had been xor-applied to answer already,
+		* so this also checks for correctness of case randomization */
 	       knot_dname_is_equal(qname, knot_pkt_qname(answer));
 }
 
@@ -1029,6 +1033,7 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
 	if (!query) {
 		return ctx->state;
 	}
+	query->flags.PKT_IS_SANE = false;
 
 	WITH_VERBOSE(query) {
 		if (query->flags.TRACE) {
@@ -1072,6 +1077,10 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
 		return KR_STATE_CONSUME;
 	}
 
+	/* If exiting above here, there's no sense to put it into packet cache.
+	 * The most important part is to check for spoofing: is_paired_to_query() */
+	query->flags.PKT_IS_SANE = true;
+
 #ifndef NOVERBOSELOG
 	const knot_lookup_t *rcode = knot_lookup_by_id(knot_rcode_names, knot_wire_get_rcode(pkt->wire));
 #endif
diff --git a/lib/resolve.c b/lib/resolve.c
index b4b3657..b771cdc 100644
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -150,6 +150,8 @@ static void randomized_qname_case(knot_dname_t * restrict qname, uint32_t secret
 	assert(qname);
 	const int len = knot_dname_size(qname) - 2; /* Skip first, last label. */
 	for (int i = 0; i < len; ++i) {
+		/* Note: this relies on the fact that correct label lengths
+		 * can't pass the isletter() test (by "luck"). */
 		if (isletter(*++qname)) {
 				*qname ^= ((secret >> (i & 31)) & 1) * 0x20;
 		}
diff --git a/lib/rplan.h b/lib/rplan.h
index 21b0b0b..7e82b03 100644
--- a/lib/rplan.h
+++ b/lib/rplan.h
@@ -64,6 +64,8 @@ struct kr_qflags {
 	bool DNS64_MARK : 1;     /**< Internal mark for dns64 module. */
 	bool CACHE_TRIED : 1;    /**< Internal to cache module. */
 	bool NO_NS_FOUND : 1;    /**< No valid NS found during last PRODUCE stage. */
+	bool PKT_IS_SANE : 1;    /**< Set by iterator in consume phase to indicate whether
+				  * some basic aspects of the packet are OK, e.g. QNAME. */
 };
 
 /** Combine flags together.  This means set union for simple flags. */