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.
This commit is contained in:
Simon Kelley committed 2026-04-21 22:14:41 +01:00
1 parent 9eb0757da9
commit 36d081e374
2 files changed
+19 -16

No files matched your search

+17 -11
View File
@@ -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)
+2 -5
View File
@@ -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)))