- For #1507: TOCTOU race in store_rrsets() can return a

CNAME chain whose target owns no address.

daemon/cachedump.c

@@ -554,7 +554,7 @@ move_into_cache(struct ub_packed_rrset_key* k,
 
 	ref.key = ak;
 	ref.id = ak->id;
-	(void)rrset_cache_update(worker->env.rrset_cache, &ref,
+	rrset_cache_update_unlock(worker->env.rrset_cache, &ref,
 		worker->alloc, *worker->env.now);
 
 	return 1;

doc/Changelog

@@ -1,3 +1,7 @@
+15 September 2026: Wouter
+	- For #1507: TOCTOU race in store_rrsets() can return a
+	  CNAME chain whose target owns no address.
+
 10 September 2026: Wouter
 	- Fix to construct the delegation point during DS NS find
 	  as a child side delegation point if that was the response.

iterator/iter_scrub.c

@@ -893,8 +893,8 @@ store_rrset(sldns_buffer* pkt, struct msg_parse* msg, struct module_env* env,
 	packed_rrset_ttl_add(d, now);
 	ref.key = k;
 	ref.id = k->id;
-	/*ignore ret: it was in the cache, ref updated */
-	(void)rrset_cache_update(env->rrset_cache, &ref, env->alloc, now);
+	/* if it was in the cache, ref updated */
+	rrset_cache_update_unlock(env->rrset_cache, &ref, env->alloc, now);
 }
 
 /**

iterator/iter_utils.c

@@ -1215,8 +1215,8 @@ iter_store_parentside_rrset(struct module_env* env,
 	rrset->entry.hash = rrset_key_hash(&rrset->rk);
 	ref.key = rrset;
 	ref.id = rrset->id;
-	/* ignore ret: if it was in the cache, ref updated */
-	(void)rrset_cache_update(env->rrset_cache, &ref, env->alloc, *env->now);
+	/* if it was in the cache, ref updated */
+	rrset_cache_update_unlock(env->rrset_cache, &ref, env->alloc, *env->now);
 }
 
 /** fetch NS record from reply, if any */

services/cache/dns.c

@@ -95,7 +95,8 @@ store_rrsets(struct module_env* env, struct reply_info* rep, time_t now,
 		case 2: /* ref updated, cache is superior */
 			if(region) {
 				struct ub_packed_rrset_key* ck;
-				lock_rw_rdlock(&rep->ref[i].key->entry.lock);
+				/* the entry is already readlocked from the
+				 * check in rrset_cache_update. */
 				/* if deleted rrset, do not copy it */
 				if(rep->ref[i].key->id == 0 ||
 					rep->ref[i].id != rep->ref[i].key->id)
@@ -116,6 +117,8 @@ store_rrsets(struct module_env* env, struct reply_info* rep, time_t now,
 						qrep->serve_expired_ttl = qrep->ttl + SERVE_EXPIRED_TTL;
 					}
 				}
+			} else {
+				lock_rw_unlock(&rep->ref[i].key->entry.lock);
 			}
 			/* no break: also copy key item */
 			/* the line below is matched by gcc regex and silences
@@ -1208,9 +1211,9 @@ dns_cache_store(struct module_env* env, struct query_info* msgqinf,
 				rep->rrsets[i]->entry.data, *env->now);
 			ref.key = rep->rrsets[i];
 			ref.id = rep->rrsets[i]->id;
-			/*ignore ret: it was in the cache, ref updated */
+			/* if it was in the cache, ref updated */
 			/* no leeway for typeNS */
-			(void)rrset_cache_update(env->rrset_cache, &ref, 
+			rrset_cache_update_unlock(env->rrset_cache, &ref,
 				env->alloc,
 				((ntohs(ref.key->rk.type)==LDNS_RR_TYPE_NS
 				 && !pside) ? qstarttime:*env->now + leeway));

services/cache/rrset.c

@@ -239,9 +239,13 @@ rrset_cache_update(struct rrset_cache* r, struct rrset_ref* ref,
 			equal, (rrset_type==LDNS_RR_TYPE_NS),
 			(rrset_type==LDNS_RR_TYPE_A || rrset_type==LDNS_RR_TYPE_AAAA))) {
 			/* cache is superior, return that value */
+			if(equal) {
+				ub_packed_rrset_parsedelete(k, alloc);
+				/* return readlocked item. */
+				return 2;
+			}
 			lock_rw_unlock(&e->lock);
 			ub_packed_rrset_parsedelete(k, alloc);
-			if(equal) return 2;
 			return 1;
 		}
 		lock_rw_unlock(&e->lock);
@@ -269,6 +273,15 @@ rrset_cache_update(struct rrset_cache* r, struct rrset_ref* ref,
 	return 0;
 }
 
+void rrset_cache_update_unlock(struct rrset_cache* r, struct rrset_ref* ref,
+	struct alloc_cache* alloc, time_t timenow)
+{
+	int ret = rrset_cache_update(r, ref, alloc, timenow);
+	if(ret == 2) {
+		lock_rw_unlock(&ref->key->entry.lock);
+	}
+}
+
 /** See if the name is a within signer authority */
 static int
 dname_subdomain_rrsig_signers(uint8_t* dname,
@@ -333,8 +346,8 @@ void rrset_cache_update_wildcard(struct rrset_cache* rrset_cache,
 	rrset->entry.hash = rrset_key_hash(&rrset->rk);
 	ref.key = rrset;
 	ref.id = rrset->id;
-	/* ignore ret: if it was in the cache, ref updated */
-	(void)rrset_cache_update(rrset_cache, &ref, alloc, timenow);
+	/* if it was in the cache, ref updated */
+	rrset_cache_update_unlock(rrset_cache, &ref, alloc, timenow);
 }
 
 /** Grace period in seconds for TTL=0 DNAME rrsets (RFC 2308: do not cache).

services/cache/rrset.h

@@ -129,10 +129,16 @@ void rrset_cache_touch(struct rrset_cache* r, struct ub_packed_rrset_key* key,
  * 	1: reference updated, item is inserted in cache.
  * 	2: reference updated, item in cache is considered superior.
  *	   also the rdata is equal (but other parameters in cache are superior).
+ *	   In case 2 the item is returned under a readlock. The caller must
+ *	   unlock.
  */
 int rrset_cache_update(struct rrset_cache* r, struct rrset_ref* ref, 
 	struct alloc_cache* alloc, time_t timenow);
 
+/** The rrset_cache_update routine that unlocks at end */
+void rrset_cache_update_unlock(struct rrset_cache* r, struct rrset_ref* ref,
+	struct alloc_cache* alloc, time_t timenow);
+
 /**
  * Update or add an rrset in the rrset cache using a wildcard dname.
  * Generates wildcard dname by prepending the wildcard label to the closest