Handle malformed query packets sensibly.

Previously, hash_questions() would return a random hash
if the packet was malformed, and probably the hash of a previous
query. Now handle this as an error.
This commit is contained in:
Simon Kelley
2022-01-09 23:21:55 +00:00
parent 8cfcd9ff63
commit 1033130b6c
2 changed files with 49 additions and 33 deletions

View File

@@ -272,6 +272,15 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
/* new query */ /* new query */
if (!forward) if (!forward)
{ {
/* If the query is malformed, we can't forward it because
we can't get a reliable hash to recognise the answer. */
if (!hash)
{
flags = 0;
ede = EDE_INVALID_DATA;
goto reply;
}
if (lookup_domain(daemon->namebuff, gotname, &first, &last)) if (lookup_domain(daemon->namebuff, gotname, &first, &last))
flags = is_local_answer(now, first, daemon->namebuff); flags = is_local_answer(now, first, daemon->namebuff);
else else
@@ -865,7 +874,9 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header,
STAT_ISEQUAL(status, STAT_NEED_KEY) ? T_DNSKEY : T_DS, server->edns_pktsz); STAT_ISEQUAL(status, STAT_NEED_KEY) ? T_DNSKEY : T_DS, server->edns_pktsz);
flags = STAT_ISEQUAL(status, STAT_NEED_KEY) ? FREC_DNSKEY_QUERY : FREC_DS_QUERY; flags = STAT_ISEQUAL(status, STAT_NEED_KEY) ? FREC_DNSKEY_QUERY : FREC_DS_QUERY;
hash = hash_questions(header, nn, daemon->namebuff);
if (!(hash = hash_questions(header, nn, daemon->namebuff)))
return;
if ((new = lookup_frec_by_query(hash, flags, FREC_DNSKEY_QUERY | FREC_DS_QUERY))) if ((new = lookup_frec_by_query(hash, flags, FREC_DNSKEY_QUERY | FREC_DS_QUERY)))
{ {
@@ -1694,13 +1705,16 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet,
unsigned char *payload = &packet[2]; unsigned char *payload = &packet[2];
struct dns_header *header = (struct dns_header *)payload; struct dns_header *header = (struct dns_header *)payload;
unsigned char c1, c2; unsigned char c1, c2;
unsigned char hash[HASH_SIZE]; unsigned char hash[HASH_SIZE], *hashp;
unsigned int rsize; unsigned int rsize;
(void)mark; (void)mark;
(void)have_mark; (void)have_mark;
memcpy(hash, hash_questions(header, (unsigned int)qsize, daemon->namebuff), HASH_SIZE); if (!(hashp = hash_questions(header, (unsigned int)qsize, daemon->namebuff)))
return 0;
memcpy(hash, hashp, HASH_SIZE);
while (1) while (1)
{ {
@@ -1781,7 +1795,7 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet,
someone might be attempting to insert bogus values into the cache by someone might be attempting to insert bogus values into the cache by
sending replies containing questions and bogus answers. sending replies containing questions and bogus answers.
Try another server, or give up */ Try another server, or give up */
if (memcmp(hash, hash_questions(header, rsize, daemon->namebuff), HASH_SIZE) != 0) if (!(hashp = hash_questions(header, rsize, daemon->namebuff)) || memcmp(hash, hashp, HASH_SIZE) != 0)
continue; continue;
serv->flags |= SERV_GOT_TCP; serv->flags |= SERV_GOT_TCP;
@@ -2547,7 +2561,8 @@ static struct frec *lookup_frec(unsigned short id, int fd, void *hash, int *firs
int first, last; int first, last;
struct randfd_list *fdl; struct randfd_list *fdl;
for(f = daemon->frec_list; f; f = f->next) if (hash)
for (f = daemon->frec_list; f; f = f->next)
if (f->sentto && f->new_id == id && if (f->sentto && f->new_id == id &&
(memcmp(hash, f->hash, HASH_SIZE) == 0)) (memcmp(hash, f->hash, HASH_SIZE) == 0))
{ {
@@ -2576,6 +2591,7 @@ static struct frec *lookup_frec_by_query(void *hash, unsigned int flags, unsigne
{ {
struct frec *f; struct frec *f;
if (hash)
for(f = daemon->frec_list; f; f = f->next) for(f = daemon->frec_list; f; f = f->next)
if (f->sentto && if (f->sentto &&
(f->flags & flagmask) == flags && (f->flags & flagmask) == flags &&

View File

@@ -55,7 +55,7 @@ unsigned char *hash_questions(struct dns_header *header, size_t plen, char *name
char *cp, c; char *cp, c;
if (!extract_name(header, plen, &p, name, 1, 4)) if (!extract_name(header, plen, &p, name, 1, 4))
break; /* bad packet */ return NULL; /* bad packet */
for (cp = name; (c = *cp); cp++) for (cp = name; (c = *cp); cp++)
if (c >= 'A' && c <= 'Z') if (c >= 'A' && c <= 'Z')
@@ -67,7 +67,7 @@ unsigned char *hash_questions(struct dns_header *header, size_t plen, char *name
p += 4; p += 4;
if (!CHECK_LEN(header, p, plen, 0)) if (!CHECK_LEN(header, p, plen, 0))
break; /* bad packet */ return NULL; /* bad packet */
} }
hash->digest(ctx, hash->digest_size, digest); hash->digest(ctx, hash->digest_size, digest);
@@ -109,7 +109,7 @@ unsigned char *hash_questions(struct dns_header *header, size_t plen, char *name
char *cp, c; char *cp, c;
if (!extract_name(header, plen, &p, name, 1, 4)) if (!extract_name(header, plen, &p, name, 1, 4))
break; /* bad packet */ return NULL; /* bad packet */
for (cp = name; (c = *cp); cp++) for (cp = name; (c = *cp); cp++)
if (c >= 'A' && c <= 'Z') if (c >= 'A' && c <= 'Z')
@@ -121,7 +121,7 @@ unsigned char *hash_questions(struct dns_header *header, size_t plen, char *name
p += 4; p += 4;
if (!CHECK_LEN(header, p, plen, 0)) if (!CHECK_LEN(header, p, plen, 0))
break; /* bad packet */ return NULL; /* bad packet */
} }
sha256_final(&ctx, digest); sha256_final(&ctx, digest);