From 77241604d38a1b0d987c9ae84d016411289a2f07 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Tue, 29 Sep 2026 09:21:04 +0200 Subject: [PATCH] BUG/MEDIUM: resolvers: properly skip root-named authority and additional records resolv_read_name() returns zero both on error and when the name is the root (a single zero byte), since it returns the length of the decoded name. When parsing the authority and additional records of an SRV response, resolv_validate_dns_response() handles a zero length by moving to the next record without advancing the read pointer. For a root name, all remaining records are then parsed again from the same place and ignored, and for an error, the following records are parsed from an unknown position. The most common root-named record is the EDNS OPT pseudo-record, which is placed in the additional section, and which RFC 6891 allows anywhere in it. When a server places it before the glue records, all of them are ignored, and the servers' addresses must be resolved with extra A/AAAA queries. An attempt to make this an error in commit fefce297a ("BUG/MEDIUM: dns: fix long loops in additional records parse on name failure") had to be reverted by commit 091768ab3 because it rejected all responses containing an OPT record. This can be reproduced with an SRV response whose additional section starts with an OPT record followed by the A glue records: A queries are sent for all targets instead of using the glue records. The srv_glue reg-test was extended to cover this case. Let's explicitly detect the root name, which is then processed as any other name (the OPT record being ignored and skipped as an unsupported type). On real errors, since we can't know where the next record starts, let's stop parsing the remaining authority and additional records instead of parsing garbage, but still keep the answers. This patch should be backported to all stable branches, after the previous fix ("BUG/MEDIUM: resolvers: skip the data of ignored additional records") which is needed for the OPT record to be skipped. --- src/resolvers.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/resolvers.c b/src/resolvers.c index 27b383f0f..db8714ac8 100644 --- a/src/resolvers.c +++ b/src/resolvers.c @@ -1478,8 +1478,12 @@ static int resolv_validate_dns_response(unsigned char *resp, unsigned char *bufe offset = 0; len = resolv_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset, 0); - if (len == 0) - continue; + + /* a zero length is either an error or the root name. We can't + * know where the next record starts in case of error. + */ + if (len == 0 && (reader >= bufend || *reader != 0)) + goto skip_parsing_additional_records; if (reader + offset + 10 > bufend) goto invalid_resp; @@ -1515,10 +1519,14 @@ static int resolv_validate_dns_response(unsigned char *resp, unsigned char *bufe offset = 0; len = resolv_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset, 0); - if (len == 0) { + /* a zero length is either an error or the root name (e.g. the + * OPT pseudo-record). We can't know where the next record + * starts in case of error. + */ + if (len == 0 && (reader >= bufend || *reader != 0)) { pool_free(resolv_answer_item_pool, answer_record); answer_record = NULL; - continue; + goto skip_parsing_additional_records; } memcpy(answer_record->name, tmpname, len); -- 2.47.3