From ff341c1861729b1521bb64c0290febe2064ae205 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Wed, 26 Feb 2014 15:54:16 +0000 Subject: [PATCH] [dns] Update end-of-name pointer after processing CNAME record Commit d4c0226 ("[dns] Support DNS search lists") introduced a regression when handling CNAME records resolving to names longer than the original name. The "end of name" offset stored in dns->offset was not updated to reflect the length of the new name, causing dns_question() to append the (empty) search suffix at an incorrect offset within the name buffer, resulting in a mangled DNS name. In the case of a CNAME record resolving to a name shorter than or equal in length to the original name, then the mangling would occur in an unused portion of the name buffer. In the common case of a name server returning the A (or AAAA) record along with the CNAME record, this would cause name resolution to succeed despite the mangling. (If the name server did not return the A or AAAA record along with the CNAME record, then the mangling would be revealed by the subsequent invalid query packet.) Reported-by: Nicolas Sylvain Signed-off-by: Michael Brown --- src/net/udp/dns.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/net/udp/dns.c b/src/net/udp/dns.c index 73af943d..fffe6e69 100644 --- a/src/net/udp/dns.c +++ b/src/net/udp/dns.c @@ -550,6 +550,9 @@ static int dns_question ( struct dns_request *dns ) { /* Restore name */ dns->name.offset = offsetof ( typeof ( dns->buf ), name ); + DBGC2 ( dns, "DNS %p question is %s type %s\n", dns, + dns_name ( &dns->name ), dns_type ( dns->question->qtype ) ); + return 0; } @@ -614,6 +617,7 @@ static int dns_xfer_deliver ( struct dns_request *dns, size_t answer_offset; size_t next_offset; size_t rdlength; + size_t name_len; int rc; /* Sanity check */ @@ -691,8 +695,12 @@ static int dns_xfer_deliver ( struct dns_request *dns, } /* Skip non-matching names */ - if ( dns_compare ( &buf, &dns->name ) != 0 ) + if ( dns_compare ( &buf, &dns->name ) != 0 ) { + DBGC2 ( dns, "DNS %p ignoring response for %s type " + "%s\n", dns, dns_name ( &buf ), + dns_type ( rr->common.type ) ); continue; + } /* Handle answer */ switch ( rr->common.type ) { @@ -745,7 +753,9 @@ static int dns_xfer_deliver ( struct dns_request *dns, DBGC ( dns, "DNS %p found CNAME %s\n", dns, dns_name ( &buf ) ); dns->search.offset = dns->search.len; - dns_copy ( &buf, &dns->name ); + name_len = dns_copy ( &buf, &dns->name ); + dns->offset = ( offsetof ( typeof ( dns->buf ), name ) + + name_len - 1 /* Strip root label */ ); if ( ( rc = dns_question ( dns ) ) != 0 ) { dns_done ( dns, rc ); goto done;