Fix caching bug with negative records.

To provoke this bug, at least the following must be true.

A reply must be a CNAME.
The target of the CNAME must not exist for the queried RR (a No Data reply).
The No Data reply must include an SOA record in the NS section.
The query must take place over TCP.

The result is the caching of a CNAME whose target is the
name of the SOA record, instead of the No data record.

If there is a RR at this target, then a subsequent query for that
RRtype will get a qrong reply.

Thanks to the testers extraordinaire at Pi-Hole for spotting this
and providing enough information to chase it down.
This commit is contained in:
Simon Kelley
2025-08-10 23:32:59 +01:00
parent c91c66ee63
commit ff30fa4b91
2 changed files with 24 additions and 11 deletions

View File

@@ -938,15 +938,14 @@ int cache_recv_insert(time_t now, int fd)
if (newc) if (newc)
{ {
newc->addr.cname.is_name_ptr = 0; newc->addr.cname.is_name_ptr = 0;
newc->addr.cname.target.cache = crecp;
if (!crecp) if (crecp)
newc->addr.cname.target.cache = NULL;
else
{ {
next_uid(crecp); next_uid(crecp);
newc->addr.cname.target.cache = crecp;
newc->addr.cname.uid = crecp->uid; newc->addr.cname.uid = crecp->uid;
} }
crecp = newc;
} }
} }
else else

View File

@@ -516,7 +516,7 @@ int do_doctor(struct dns_header *header, size_t qlen, char *namebuff)
Cache said SOA and return the difference in length between name and the name of the Cache said SOA and return the difference in length between name and the name of the
SOA RR so we can look it up again. SOA RR so we can look it up again.
*/ */
static int find_soa(struct dns_header *header, size_t qlen, char *name, int *substring, unsigned long *ttlp, int no_cache, time_t now) static int find_soa(struct dns_header *header, size_t qlen, char *name, int *substring, unsigned long *ttlp, int cache, time_t now)
{ {
unsigned char *p, *psave; unsigned char *p, *psave;
int qtype, qclass, rdlen; int qtype, qclass, rdlen;
@@ -556,7 +556,7 @@ static int find_soa(struct dns_header *header, size_t qlen, char *name, int *sub
{ {
int prefix = name_len - soa_len; int prefix = name_len - soa_len;
if (!no_cache) if (cache)
{ {
if (!(addr.rrblock.rrdata = blockdata_alloc(NULL, 0))) if (!(addr.rrblock.rrdata = blockdata_alloc(NULL, 0)))
return 0; return 0;
@@ -568,12 +568,12 @@ static int find_soa(struct dns_header *header, size_t qlen, char *name, int *sub
{ {
if (!extract_name(header, qlen, &p, daemon->workspacename, EXTR_NAME_EXTRACT, 0)) if (!extract_name(header, qlen, &p, daemon->workspacename, EXTR_NAME_EXTRACT, 0))
{ {
if (!no_cache) if (cache)
blockdata_free(addr.rrblock.rrdata); blockdata_free(addr.rrblock.rrdata);
return 0; return 0;
} }
if (!no_cache) if (cache)
{ {
len = to_wire(daemon->workspacename); len = to_wire(daemon->workspacename);
if (!blockdata_expand(addr.rrblock.rrdata, addr.rrblock.datalen, daemon->workspacename, len)) if (!blockdata_expand(addr.rrblock.rrdata, addr.rrblock.datalen, daemon->workspacename, len))
@@ -588,13 +588,13 @@ static int find_soa(struct dns_header *header, size_t qlen, char *name, int *sub
if (!CHECK_LEN(header, p, qlen, 20)) if (!CHECK_LEN(header, p, qlen, 20))
{ {
if (!no_cache) if (cache)
blockdata_free(addr.rrblock.rrdata); blockdata_free(addr.rrblock.rrdata);
return 0; return 0;
} }
/* rest of RR */ /* rest of RR */
if (!no_cache) if (cache)
{ {
int secflag = 0; int secflag = 0;
@@ -1079,7 +1079,17 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
if (insert && !option_bool(OPT_NO_NEG)) if (insert && !option_bool(OPT_NO_NEG))
{ {
int substring, have_soa = find_soa(header, qlen, name, &substring, &ttl, no_cache_dnssec, now); /* The order of records going into the cache matters (see cache_recv_insert()).
The target of a CNAME must immediately follow the CNAME.
(CNAME has already gone into the cache at this point)
Here we call find_soa to get the ttl and substring, but
we DON'T LET IT INSERT the SOA into the cache if our negative record is a CNAME target
so that the SOA doesn't come before the CNAME target.
We call find_soa() again after inserting the CNAME target to insert the SOA
if necessary. */
int substring, have_soa = find_soa(header, qlen, name, &substring, &ttl, cpp == NULL, now);
if (have_soa || daemon->neg_ttl) if (have_soa || daemon->neg_ttl)
{ {
@@ -1101,6 +1111,10 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
next_uid(newc); next_uid(newc);
cpp->addr.cname.target.cache = newc; cpp->addr.cname.target.cache = newc;
cpp->addr.cname.uid = newc->uid; cpp->addr.cname.uid = newc->uid;
/* we didn't insert the SOA before a CNAME target above, do it now. */
if (have_soa)
find_soa(header, qlen, name, NULL, NULL, 1, now);
} }
} }
} }