fix(dns): resolve CNAME chains and prevent invalid IP parsing

- Add automated in-band CNAME chain resolution to extract final A/AAAA IP addresses. - Replace `MaxResponseAnswers` with `MaxIPs` and `MaxCNAMEs` to explicitly bound resource decoding and prevent memory exhaustion. - Bound CNAME chain traversal to prevent infinite loops from cyclic records.
2026-08-31 11:59:05 +00:00 · 2026-08-20 22:38:28 +09:00
parent 263b1ecf11
commit a33e9c5fab
5 changed files with 328 additions and 35 deletions
@@ -24,9 +24,8 @@ type ResolveConfig struct {
 	Questions       []Question
 	Additional      []Resource
 	EnableRecursion bool
-	// MaxResponseAnswers limits how many answer records are decoded from the
-	// DNS response. If zero it defaults to the number of Questions.
-	MaxResponseAnswers uint16
+	MaxIPs          uint16
+	MaxCNAMEs       uint16
 }

 func (sudp *Client) Protocol() uint64 { return uint64(lneto.IPProtoUDP) }
@@ -37,13 +36,18 @@ func (sudp *Client) ConnectionID() *uint64 { return &sudp.connID }

 func (c *Client) StartResolve(localPort, txid uint16, cfg ResolveConfig) error {
 	nd := len(cfg.Questions)
-	if nd > math.MaxUint16 {
+	if nd > math.MaxUint16 || nd == 0 {
 		return lneto.ErrInvalidConfig
 	}
-	maxAns := cfg.MaxResponseAnswers
-	if maxAns == 0 {
-		maxAns = uint16(nd)
+	maxIPs := cfg.MaxIPs
+	if maxIPs == 0 {
+		maxIPs = uint16(nd)
 	}
+	maxCNAMEs := cfg.MaxCNAMEs
+	if maxCNAMEs == 0 {
+		maxCNAMEs = 16
+	}
+	maxAns := maxIPs + maxCNAMEs
 	c.reset(localPort, txid, CQueryPending, cfg.EnableRecursion)
 	c.msg.LimitResourceDecoding(uint16(nd), maxAns, 0, 0)
 	c.msg.AddQuestions(cfg.Questions)
@@ -48,6 +48,7 @@ type Question struct {
 type Resource struct {
 	header ResourceHeader
 	data   []byte
+	target Name
 }

 // A ResourceHeader is the header of a DNS resource record. There are
@@ -300,22 +301,45 @@ func (m *Message) AppendTo(buf []byte, txid uint16, flags HeaderFlags) (_ []byte
 }

 func (m *Message) WriteAnswers(dst []netip.Addr, host string) (n uint16, err error) {
-	for i := range m.Answers {
-		if int(n) >= len(dst) {
-			return n, lneto.ErrExhausted
+	var alias Name
+	// Each pass follows at most one alias hop
+	// The pass limit also bounds CNAME cycles in malformed responses.
+	for range m.Answers {
+		var next Name
+		n = 0
+		for i := range m.Answers {
+			if int(n) >= len(dst) {
+				break
+			}
+			ans := &m.Answers[i]
+			hdr := ans.Header()
+			matched := alias.Len() == 0 && hdr.Name.EqualString(host) ||
+				alias.Len() != 0 && NamesEqual(hdr.Name, alias)
+			if !matched {
+				continue
+			}
+			switch hdr.Type {
+			case TypeA, TypeAAAA:
+				var ok bool
+				dst[n], ok = netip.AddrFromSlice(ans.RawData())
+				if !ok {
+					err = lneto.ErrInvalidAddr
+					continue
+				} else {
+					n++
+				}
+			case TypeCNAME:
+				if ans.target.Len() != 0 {
+					next = ans.target
+				}
+			}
 		}
-		ans := &m.Answers[i]
-		hdr := ans.Header()
-		if !hdr.Name.EqualString(host) {
-			continue
-		}
-		var ok bool
-		dst[n], ok = netip.AddrFromSlice(ans.RawData())
-		if !ok {
-			err = lneto.ErrInvalidAddr
-		} else {
-			n++
+		if next.Len() == 0 || n > 0 {
+			// Either no new alias was found or addresses were resolved for
+			// the current name; a name cannot alias and hold records at once.
+			break
 		}
+		alias = next
 	}
 	return n, err
 }
@@ -399,6 +423,7 @@ func (h *ResourceHeader) String() string {
 func (r *Resource) Reset() {
 	r.header.Reset()
 	r.data = r.data[:0]
+	r.target.Reset()
 }

 func (r *Resource) Header() ResourceHeader { return r.header }
@@ -460,6 +485,14 @@ func (r *Resource) Decode(b []byte, off uint16) (uint16, error) {
 	if r.header.Length > uint16(len(b[off:])) {
 		return off, errResourceLen
 	}
+	if r.header.Type == TypeCNAME {
+		_, err = r.target.Decode(b, off)
+		if err != nil {
+			r.target.Reset() // Tolerate undecodable target; CNAME chain lookup skips it.
+		}
+	} else {
+		r.target.Reset()
+	}
 	r.data = append(r.data[:0], b[off:off+r.header.Length]...)
 	return off + r.header.Length, nil
 }
@@ -775,6 +808,7 @@ func (dst *Question) CopyFrom(q Question) {
 func (dst *Resource) CopyFrom(r Resource) {
 	dst.header.CopyFrom(r.header)
 	dst.data = append(dst.data[:0], r.data...)
+	dst.target.CopyFrom(r.target)
 }

 // SetA sets an A (IPv4 address) resource record, reusing internal buffers.
@@ -239,26 +239,178 @@ func TestDecodeMessage(t *testing.T) {
 	}
 }

+// Regression test for CNAME-following: a response for www.yahoo.co.jp
+// contains a CNAME record to edge12.g.yimg.jp (with compressed labels in its
+// RDATA) followed by the A record for the canonical name. The CNAME RDATA
+// must not be interpreted as an IP address and the A record must be returned.
+func TestClient_CNAMEResponse(t *testing.T) {
+	const hostname = "www.yahoo.co.jp"
+	const txid = uint16(0x1234)
+	const clientPort = uint16(54321)
+	response := []byte{
+		// Header: txid 0x1234, QR|RD|RA, QD=1 AN=2 NS=0 AR=0.
+		0x12, 0x34, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00,
+		// Question: www.yahoo.co.jp A IN.
+		0x03, 'w', 'w', 'w', 0x05, 'y', 'a', 'h', 'o', 'o', 0x02, 'c', 'o', 0x02, 'j', 'p', 0x00,
+		0x00, 0x01, 0x00, 0x01,
+		// Answer 1: (ptr to question) CNAME IN ttl=842 rdlen=16
+		// rdata: edge12.g.yimg.jp with "jp" as compression pointer to offset 0x19.
+		0xc0, 0x0c, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x03, 0x4a, 0x00, 0x10,
+		0x06, 'e', 'd', 'g', 'e', '1', '2', 0x01, 'g', 0x04, 'y', 'i', 'm', 'g', 0xc0, 0x19,
+		// Answer 2: (ptr into CNAME rdata) A IN ttl=36 rdlen=4 182.22.23.124.
+		0xc0, 0x2d, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x04, 0xb6, 0x16, 0x17, 0x7c,
+	}
+	name := MustNewName(hostname)
+	var client Client
+	err := client.StartResolve(clientPort, txid, ResolveConfig{
+		Questions: []Question{{
+			Name:  name,
+			Type:  TypeA,
+			Class: ClassINET,
+		}},
+		EnableRecursion: true,
+		MaxIPs:          4,
+		MaxCNAMEs:       2,
+	})
+	if err != nil {
+		t.Fatal("failed to start DNS resolve:", err)
+	}
+	var queryBuf [512]byte
+	_, err = client.Encapsulate(queryBuf[:], 0, 0)
+	if err != nil {
+		t.Fatal("failed to encapsulate DNS query:", err)
+	}
+	if err := client.Demux(response, 0); err != nil {
+		t.Fatal("failed to demux DNS response:", err)
+	}
+	var addrs [4]netip.Addr
+	n, err := client.ResponseAnswerLookup(addrs[:], hostname)
+	if err != nil {
+		t.Fatal("failed to look up DNS response answers:", err)
+	}
+	if n != 1 {
+		t.Fatalf("expected 1 answer, got %d: %v", n, addrs[:n])
+	}
+	if addrs[0] != (netip.AddrFrom4([4]byte{182, 22, 23, 124})) {
+		t.Fatalf("expected 182.22.23.124, got %v", addrs[0])
+	}
+}
+
+// Table-driven tests for Message.WriteAnswers covering answer reordering,
+// non-address record types and cyclic CNAME aliases.
+func TestMessage_WriteAnswers(t *testing.T) {
+	tests := []struct {
+		name     string
+		host     string
+		response []byte
+		want     []netip.Addr
+	}{
+		{
+			name: "A record before its CNAME",
+			host: "www.yahoo.co.jp",
+			// Answer 1 is the A record for edge12.g.yimg.jp, spelled out with
+			// a trailing compression pointer to "jp" in the question. Answer 2
+			// is the CNAME from www.yahoo.co.jp whose RDATA is a single
+			// backward compression pointer to answer 1's owner name.
+			response: []byte{
+				// Header: txid 0x1234, QR|RD|RA, QD=1 AN=2 NS=0 AR=0.
+				0x12, 0x34, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00,
+				// Question: www.yahoo.co.jp A IN.
+				0x03, 'w', 'w', 'w', 0x05, 'y', 'a', 'h', 'o', 'o', 0x02, 'c', 'o', 0x02, 'j', 'p', 0x00,
+				0x00, 0x01, 0x00, 0x01,
+				// Answer 1: edge12.g.yimg.jp A IN ttl=36 rdlen=4 182.22.23.124.
+				0x06, 'e', 'd', 'g', 'e', '1', '2', 0x01, 'g', 0x04, 'y', 'i', 'm', 'g', 0xc0, 0x19,
+				0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x04, 0xb6, 0x16, 0x17, 0x7c,
+				// Answer 2: (ptr to question) CNAME IN ttl=842 rdlen=2, target
+				// is a pointer to answer 1's owner name at offset 0x21.
+				0xc0, 0x0c, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x03, 0x4a, 0x00, 0x02, 0xc0, 0x21,
+			},
+			want: []netip.Addr{netip.AddrFrom4([4]byte{182, 22, 23, 124})},
+		},
+		{
+			name: "ignores non-address records",
+			host: "example.com",
+			response: []byte{
+				// Header: txid 0x5678, QR|RD|RA, QD=1 AN=2 NS=0 AR=0.
+				0x56, 0x78, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00,
+				// Question: example.com A IN.
+				0x07, 'e', 'x', 'a', 'm', 'p', 'l', 'e', 0x03, 'c', 'o', 'm', 0x00,
+				0x00, 0x01, 0x00, 0x01,
+				// Answer 1: (ptr to question) TXT IN ttl=60 rdlen=6 "hello".
+				0xc0, 0x0c, 0x00, 0x10, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x06, 0x05, 'h', 'e', 'l', 'l', 'o',
+				// Answer 2: (ptr to question) A IN ttl=300 rdlen=4 192.0.2.1.
+				0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x01, 0x2c, 0x00, 0x04, 0xc0, 0x00, 0x02, 0x01,
+			},
+			want: []netip.Addr{netip.AddrFrom4([4]byte{192, 0, 2, 1})},
+		},
+		{
+			name: "CNAME cycle terminates",
+			host: "a.com",
+			response: []byte{
+				// Header: txid 0xabcd, QR|RD|RA, QD=1 AN=2 NS=0 AR=0.
+				0xab, 0xcd, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00,
+				// Question: a.com A IN.
+				0x01, 'a', 0x03, 'c', 'o', 'm', 0x00, 0x00, 0x01, 0x00, 0x01,
+				// Answer 1: a.com CNAME b.com.
+				0x01, 'a', 0x03, 'c', 'o', 'm', 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x07, 0x01, 'b', 0x03, 'c', 'o', 'm', 0x00,
+				// Answer 2: b.com CNAME a.com.
+				0x01, 'b', 0x03, 'c', 'o', 'm', 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x07, 0x01, 'a', 0x03, 'c', 'o', 'm', 0x00,
+			},
+			want: nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var msg Message
+			msg.LimitResourceDecoding(1, 4, 0, 0)
+			_, incomplete, err := msg.Decode(tt.response)
+			if incomplete || err != nil {
+				t.Fatal("decode:", incomplete, err)
+			}
+			var addrs [4]netip.Addr
+			n, err := msg.WriteAnswers(addrs[:], tt.host)
+			if err != nil {
+				t.Fatal("write answers:", err)
+			}
+			if n != uint16(len(tt.want)) {
+				t.Fatalf("expected %d addresses, got %d: %v", len(tt.want), n, addrs[:n])
+			}
+			for i, want := range tt.want {
+				if addrs[i] != want {
+					t.Errorf("address %d: expected %v, got %v", i, want, addrs[i])
+				}
+			}
+		})
+	}
+}
+
 func TestClient_ReceivesDNSResponse(t *testing.T) {
 	const hostname = "example.com"
 	const txid = uint16(12345)
 	const clientPort = uint16(54321)
-	const maxAnswers = 4
-	allIPs := [5][4]byte{
+	const maxIPs = 4
+	const maxCNAMEs = 2
+	const maxDecoded = maxIPs + maxCNAMEs
+	allIPs := [8][4]byte{
 		{192, 0, 2, 1},
 		{192, 0, 2, 2},
 		{192, 0, 2, 3},
 		{192, 0, 2, 4},
 		{192, 0, 2, 5},
+		{192, 0, 2, 6},
+		{192, 0, 2, 7},
+		{192, 0, 2, 8},
 	}
 	tests := []struct {
 		name        string
 		responseIPs [][4]byte
-		wantAnswers int
+		wantDecoded int // Answers decoded (and copied by ResponseCopyTo).
+		wantAnswers int // Addresses returned by ResponseAnswerLookup.
 	}{
-		{name: "single_answer", responseIPs: allIPs[:1], wantAnswers: 1},
-		{name: "multiple_answers", responseIPs: allIPs[:4], wantAnswers: 4},
-		{name: "answer_limit", responseIPs: allIPs[:5], wantAnswers: maxAnswers},
+		{name: "single_answer", responseIPs: allIPs[:1], wantDecoded: 1, wantAnswers: 1},
+		{name: "multiple_answers", responseIPs: allIPs[:4], wantDecoded: 4, wantAnswers: 4},
+		{name: "answer_limit", responseIPs: allIPs[:5], wantDecoded: 5, wantAnswers: maxIPs},
+		{name: "decode_limit", responseIPs: allIPs[:8], wantDecoded: maxDecoded, wantAnswers: maxIPs},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -290,8 +442,9 @@ func TestClient_ReceivesDNSResponse(t *testing.T) {
 					Type:  TypeA,
 					Class: ClassINET,
 				}},
-				EnableRecursion:    true,
-				MaxResponseAnswers: maxAnswers,
+				EnableRecursion: true,
+				MaxIPs:          maxIPs,
+				MaxCNAMEs:       maxCNAMEs,
 			})
 			if err != nil {
 				t.Fatal("failed to start DNS resolve:", err)
@@ -307,12 +460,12 @@ func TestClient_ReceivesDNSResponse(t *testing.T) {
 				t.Fatal("failed to demux DNS response:", err)
 			}

-			var addrs [maxAnswers]netip.Addr
+			var addrs [maxIPs]netip.Addr
 			answers, err := client.ResponseAnswerLookup(addrs[:], hostname)
 			if err != nil {
 				t.Fatal("failed to look up DNS response answers:", err)
 			}
-			if answers != uint16(tt.wantAnswers) {
+			if int(answers) != tt.wantAnswers {
 				t.Fatalf("expected %d answers, got %d", tt.wantAnswers, answers)
 			}
 			for i := 0; i < tt.wantAnswers; i++ {
@@ -334,8 +487,8 @@ func TestClient_ReceivesDNSResponse(t *testing.T) {
 			if !done {
 				t.Fatal("expected done=true")
 			}
-			if len(lookup.Answers) != tt.wantAnswers {
-				t.Fatalf("expected %d copied answers, got %d", tt.wantAnswers, len(lookup.Answers))
+			if len(lookup.Answers) != tt.wantDecoded {
+				t.Fatalf("expected %d copied answers, got %d", tt.wantDecoded, len(lookup.Answers))
 			}
 		})
 	}
@@ -629,8 +629,8 @@ func (s *StackAsync) StartLookupIPType(host string, qtype dns.Type) error {
 		Additional: []dns.Resource{
 			s.ednsopt,
 		},
-		EnableRecursion:    true,
-		MaxResponseAnswers: uint16(len(s.addrbufnip)),
+		EnableRecursion: true,
+		MaxIPs:          uint16(len(s.addrbufnip)),
 	})
 	if err != nil {
 		return err
@@ -156,6 +156,14 @@ func buildDNSResponsePacket(t *testing.T, txid uint16, dstPort uint16, hostname
 			dns.NewResource(name, dns.TypeA, dns.ClassINET, 300, addr.AsSlice()),
 		},
 	}
+	return buildDNSMsgResponsePacket(t, txid, dstPort, msg, srcIP, srcMAC, dstIP, dstMAC, buf)
+}
+
+// buildDNSMsgResponsePacket wraps a DNS response message into a complete
+// Ethernet+IP+UDP packet with valid checksums.
+func buildDNSMsgResponsePacket(t *testing.T, txid uint16, dstPort uint16, msg dns.Message,
+	srcIP netip.Addr, srcMAC [6]byte, dstIP netip.Addr, dstMAC [6]byte, buf []byte) ([]byte, error) {
+	t.Helper()

 	// Response flags: QR=1 (response), RD=1 (recursion desired), RA=1 (recursion available).
 	responseFlags := dns.HeaderFlags(1<<15 | 1<<8 | 1<<7)
@@ -237,3 +245,97 @@ var errBaseLenDNS = func() error {
 }()

 var errInvalidEtherType = errors.New("invalid ethernet type")
+
+// TestDNS_CNAMEResponse verifies that a DNS response containing a CNAME record
+// followed by an A record for the canonical name resolves to the A record's
+// address: the CNAME RDATA must not be misinterpreted as an IP address.
+func TestDNS_CNAMEResponse(t *testing.T) {
+	const seed = 9876
+	const MTU = ethernet.MaxMTU
+
+	client := new(StackAsync)
+	dnsServerAddr := netip.AddrFrom4([4]byte{8, 8, 8, 8})
+	clientAddr := netip.AddrFrom4([4]byte{10, 0, 0, 100})
+	clientMAC := [6]byte{0xde, 0xad, 0xbe, 0xef, 0x00, 0x01}
+	dnsServerMAC := [6]byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55}
+
+	err := client.Reset(StackConfig{
+		Hostname:        "DNSClient",
+		RandSeed:        seed,
+		StaticAddress4:  clientAddr.As4(),
+		DNSServer:       dnsServerAddr,
+		HardwareAddress: clientMAC,
+		MTU:             uint16(MTU),
+	})
+	if err != nil {
+		t.Fatal("client Reset failed:", err)
+	}
+	client.SetGatewayHardwareAddr(dnsServerMAC)
+
+	const hostname = "www.example.com"
+	const alias = "cdn.example.net"
+	wantAddr := netip.MustParseAddr("192.0.2.200")
+
+	err = client.StartLookupIP(hostname)
+	if err != nil {
+		t.Fatal("StartLookupIP failed:", err)
+	}
+
+	const carrierDataSize = ethernet.MaxFrameLength
+	var buf [carrierDataSize]byte
+
+	// Client sends DNS query for www.example.com.
+	n, err := client.EgressEthernet(buf[:])
+	if err != nil || n == 0 {
+		t.Fatal("expected DNS query packet from client:", err, n)
+	}
+	txid, clientPort, err := extractDNSTxIDAndPort(buf[:n])
+	if err != nil {
+		t.Fatal("failed to extract DNS txid:", err)
+	}
+
+	// Respond with a CNAME record www.example.com -> cdn.example.net
+	// followed by the A record for cdn.example.net.
+	owner, err := dns.NewName(hostname)
+	if err != nil {
+		t.Fatal(err)
+	}
+	aliasName, err := dns.NewName(alias)
+	if err != nil {
+		t.Fatal(err)
+	}
+	aliasWire, err := aliasName.AppendTo(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	msg := dns.Message{
+		Questions: []dns.Question{{
+			Name:  owner,
+			Type:  dns.TypeA,
+			Class: dns.ClassINET,
+		}},
+		Answers: []dns.Resource{
+			dns.NewResource(owner, dns.TypeCNAME, dns.ClassINET, 300, aliasWire),
+			dns.NewResource(aliasName, dns.TypeA, dns.ClassINET, 300, wantAddr.AsSlice()),
+		},
+	}
+	responsePkt, err := buildDNSMsgResponsePacket(t, txid, clientPort, msg,
+		dnsServerAddr, dnsServerMAC, clientAddr, clientMAC, buf[:])
+	if err != nil {
+		t.Fatal("failed to build CNAME response packet:", err)
+	}
+	if err = client.IngressEthernet(responsePkt); err != nil {
+		t.Fatal("client Demux of CNAME response failed:", err)
+	}
+
+	addrs, done, err := client.ResultLookupIP(hostname)
+	if err != nil {
+		t.Fatal("ResultLookupIP error:", err)
+	}
+	if !done {
+		t.Fatal("DNS lookup not done after receiving CNAME response")
+	}
+	if !slices.Contains(addrs, wantAddr) {
+		t.Errorf("expected address %s not found in result %v", wantAddr, addrs)
+	}
+}