Browse Source

[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>
tags/v1.20.1
Michael Brown 10 years ago
parent
commit
ff341c1861
1 changed files with 12 additions and 2 deletions
  1. 12
    2
      src/net/udp/dns.c

+ 12
- 2
src/net/udp/dns.c View File

550
 	/* Restore name */
550
 	/* Restore name */
551
 	dns->name.offset = offsetof ( typeof ( dns->buf ), name );
551
 	dns->name.offset = offsetof ( typeof ( dns->buf ), name );
552
 
552
 
553
+	DBGC2 ( dns, "DNS %p question is %s type %s\n", dns,
554
+		dns_name ( &dns->name ), dns_type ( dns->question->qtype ) );
555
+
553
 	return 0;
556
 	return 0;
554
 }
557
 }
555
 
558
 
614
 	size_t answer_offset;
617
 	size_t answer_offset;
615
 	size_t next_offset;
618
 	size_t next_offset;
616
 	size_t rdlength;
619
 	size_t rdlength;
620
+	size_t name_len;
617
 	int rc;
621
 	int rc;
618
 
622
 
619
 	/* Sanity check */
623
 	/* Sanity check */
691
 		}
695
 		}
692
 
696
 
693
 		/* Skip non-matching names */
697
 		/* Skip non-matching names */
694
-		if ( dns_compare ( &buf, &dns->name ) != 0 )
698
+		if ( dns_compare ( &buf, &dns->name ) != 0 ) {
699
+			DBGC2 ( dns, "DNS %p ignoring response for %s type "
700
+				"%s\n", dns, dns_name ( &buf ),
701
+				dns_type ( rr->common.type ) );
695
 			continue;
702
 			continue;
703
+		}
696
 
704
 
697
 		/* Handle answer */
705
 		/* Handle answer */
698
 		switch ( rr->common.type ) {
706
 		switch ( rr->common.type ) {
745
 			DBGC ( dns, "DNS %p found CNAME %s\n",
753
 			DBGC ( dns, "DNS %p found CNAME %s\n",
746
 			       dns, dns_name ( &buf ) );
754
 			       dns, dns_name ( &buf ) );
747
 			dns->search.offset = dns->search.len;
755
 			dns->search.offset = dns->search.len;
748
-			dns_copy ( &buf, &dns->name );
756
+			name_len = dns_copy ( &buf, &dns->name );
757
+			dns->offset = ( offsetof ( typeof ( dns->buf ), name ) +
758
+					name_len - 1 /* Strip root label */ );
749
 			if ( ( rc = dns_question ( dns ) ) != 0 ) {
759
 			if ( ( rc = dns_question ( dns ) ) != 0 ) {
750
 				dns_done ( dns, rc );
760
 				dns_done ( dns, rc );
751
 				goto done;
761
 				goto done;

Loading…
Cancel
Save