[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 <nsylvain@gmail.com> Signed-off-by: Michael Brown <mcb30@ipxe.org>
ipxe · Feb 26, 2014 · ff341c1 · ff341c1
1 parent d4c0226
commit ff341c1
Showing 1 changed file with 12 additions and 2 deletions.
diff --git 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;