From 36d081e37477027fd721fea498f3760f529034ad Mon Sep 17 00:00:00 2001
From: Simon Kelley <simon@thekelleys.org.uk>
Date: Tue, 21 Apr 2026 22:14:41 +0100
Subject: [PATCH] Fix buffer overlow in log_query()

The addition of "(not supported)" to logs of DS and DNSKEY replies
overflows the buffer used to construct the string.

Re-arrange things to avoid this, and add checks to avoid the same  problem
if the logging calls change in the future.

Thanks to Yiwei Hou for finding this.

The problem exists is DNSSEC is enabled and query logging is also
enabled. The overwrite is of bounded length and the bytes
written are not in control of an attacker, so this is not considered
a likely remote-execution vector.
---
 src/cache.c  | 28 +++++++++++++++++-----------
 src/dnssec.c |  7 ++-----
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 43c743c..f85b7a8 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -2331,7 +2331,7 @@ void log_query(unsigned int flags, char *name, union all_addr *addr, char *arg,
     return;
 
   /* build query type string if requested */
-  if (!(flags & (F_SERVER | F_IPSET | F_QUERY)) && type > 0)
+  if (!(flags & (F_SERVER | F_IPSET | F_QUERY | F_KEYTAG | F_RR)) && type > 0)
     arg = querystr(arg, type);
 
   dest = arg;
@@ -2347,19 +2347,25 @@ void log_query(unsigned int flags, char *name, union all_addr *addr, char *arg,
     {
       dest = daemon->addrbuff;
 
-       if (flags & F_RR)
-	 {
-	   if (flags & F_KEYTAG)
-	     dest = querystr(NULL, addr->rrblock.rrtype);
-	   else
-	     dest = querystr(NULL, addr->rrdata.rrtype);
-	 }
-       else if (flags & F_KEYTAG)
-	sprintf(daemon->addrbuff, arg, addr->log.keytag, addr->log.algo, addr->log.digest);
+      if (flags & F_RR)
+	{
+	  if (flags & F_KEYTAG)
+	    dest = querystr(NULL, addr->rrblock.rrtype);
+	  else
+	    dest = querystr(NULL, addr->rrdata.rrtype);
+	}
+#ifdef HAVE_DNSSEC
+      else if (flags & F_KEYTAG)
+	{
+	  snprintf(daemon->addrbuff, ADDRSTRLEN, arg, addr->log.keytag, addr->log.algo, addr->log.digest);
+	  if (type)
+	    extra = " (not supported)";
+	}
+#endif
       else if (flags & F_RCODE)
 	{
 	  unsigned int rcode = addr->log.rcode;
-
+	  
 	  if (rcode == SERVFAIL)
 	    dest = "SERVFAIL";
 	  else if (rcode == REFUSED)
diff --git a/src/dnssec.c b/src/dnssec.c
index 8a163a0..5cc37ac 100644
--- a/src/dnssec.c
+++ b/src/dnssec.c
@@ -950,10 +950,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch
 			   
 			   a.log.keytag = keytag;
 			   a.log.algo = algo;
-			   if (algo_digest_name(algo))
-			     log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DNSKEY keytag %hu, algo %hu", 0);
-			   else
-			     log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DNSKEY keytag %hu, algo %hu (not supported)", 0);
+			   log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DNSKEY keytag %hu, algo %hu", !algo_digest_name(algo));
 			 }
 		     }
 				  
@@ -1101,7 +1098,7 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char
 		  a.log.keytag = keytag;
 		  a.log.algo = algo;
 		  a.log.digest = digest;
-		  log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DS for keytag %hu, algo %hu, digest %hu (not supported)", 0);
+		  log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DS for keytag %hu, algo %hu, digest %hu", 1);
 		  neg_ttl = ttl;
 		} 
 	      else if ((key = blockdata_alloc((char*)p, rdlen - 4)))