From a778adbb1be568821b913216a8890953b9cb96bd Mon Sep 17 00:00:00 2001 From: Erick Cestari Date: Fri, 19 Jun 2026 16:50:53 -0300 Subject: [PATCH 1/2] discovery: fix panic in DNS fallback SRV lookup The fallback SRV lookup type-asserted each DNS Answer record to *dns.SRV unconditionally. If the response contains a non-SRV record (e.g. an A or CNAME), the type assertion panics and crashes the daemon. Use the comma-ok form to skip non-SRV records instead. Also guard against an empty LookupHost result for the shim, which would otherwise panic on an out-of-bounds index into addrs. This is safe to discuss and fix in public. The bug is very unlikely to be exploitable: triggering it requires either a DNS seeder to serve a malformed response, or an on-path MITM injecting one (the fallback response is unauthenticated). A malicious seeder already has far more direct ways to disrupt a node, and a MITM attack is hard to mount, so the panic does not meaningfully widen the attack surface. (cherry picked from commit 2a3642c691e08c2115f9177251bb9cbeed5f0a6a) --- discovery/bootstrapper.go | 40 +++++- discovery/bootstrapper_test.go | 217 +++++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+), 5 deletions(-) diff --git a/discovery/bootstrapper.go b/discovery/bootstrapper.go index 28ef0d920be..73fb2e7d490 100644 --- a/discovery/bootstrapper.go +++ b/discovery/bootstrapper.go @@ -382,6 +382,11 @@ func (d *DNSSeedBootstrapper) fallBackSRVLookup(soaShim string, return nil, err } + if len(addrs) == 0 { + return nil, fmt.Errorf("no addresses for fallback DNS seed "+ + "shim %v", soaShim) + } + // Once we have the IP address, we'll establish a TCP connection using // port 53. dnsServer := net.JoinHostPort(addrs[0], "53") @@ -389,6 +394,7 @@ func (d *DNSSeedBootstrapper) fallBackSRVLookup(soaShim string, if err != nil { return nil, err } + _ = conn.SetDeadline(time.Now().Add(d.timeout)) dnsHost := fmt.Sprintf("_nodes._tcp.%v.", targetEndPoint) dnsConn := &dns.Conn{Conn: conn} @@ -416,7 +422,18 @@ func (d *DNSSeedBootstrapper) fallBackSRVLookup(soaShim string, // that net.LookupSRV would normally return. var rrs []*net.SRV for _, rr := range resp.Answer { - srv := rr.(*dns.SRV) + // The answer section may contain records other than SRV + // (e.g. A or CNAME), so use the comma-ok form to skip any + // non-SRV record instead of panicking on a failed type + // assertion. + srv, ok := rr.(*dns.SRV) + if !ok { + log.Infof("Skipping non-SRV record %T in fallback "+ + "DNS seed response for %v", rr, targetEndPoint) + + continue + } + rrs = append(rrs, &net.SRV{ Target: srv.Target, Port: srv.Port, @@ -425,6 +442,11 @@ func (d *DNSSeedBootstrapper) fallBackSRVLookup(soaShim string, }) } + if len(rrs) == 0 { + return nil, fmt.Errorf("no SRV records in fallback DNS seed "+ + "response for %v", targetEndPoint) + } + return rrs, nil } @@ -498,7 +520,9 @@ search: bechNodeHost := nodeSrv.Target addrs, err := d.net.LookupHost(bechNodeHost) if err != nil { - return nil, err + log.Tracef("Skipping node %v: %v", + bechNodeHost, err) + continue } if len(addrs) == 0 { @@ -523,7 +547,9 @@ search: bechNode := strings.Split(bechNodeHost, ".") _, nodeBytes5Bits, err := bech32.Decode(bechNode[0]) if err != nil { - return nil, err + log.Tracef("Skipping node %v: %v", + bechNodeHost, err) + continue } // Once we have the bech32 decoded pubkey, we'll need @@ -534,11 +560,15 @@ search: nodeBytes5Bits, 5, 8, false, ) if err != nil { - return nil, err + log.Tracef("Skipping node %v: %v", + bechNodeHost, err) + continue } nodeKey, err := btcec.ParsePubKey(nodeBytes) if err != nil { - return nil, err + log.Tracef("Skipping node %v: %v", + bechNodeHost, err) + continue } // If we have an ignore list, and this node is in the diff --git a/discovery/bootstrapper_test.go b/discovery/bootstrapper_test.go index f01e67bc759..5101fb00d8f 100644 --- a/discovery/bootstrapper_test.go +++ b/discovery/bootstrapper_test.go @@ -2,12 +2,15 @@ package discovery import ( "context" + "fmt" "net" "testing" + "time" "github.com/btcsuite/btcd/btcec/v2" "github.com/lightningnetwork/lnd/autopilot" "github.com/lightningnetwork/lnd/tor" + "github.com/miekg/dns" "github.com/stretchr/testify/require" ) @@ -55,6 +58,60 @@ func (s *stubChannelGraph) ForEachNodesChannels(_ context.Context, return nil } +// fallbackNet is a tor.Net stub used to drive fallBackSRVLookup. LookupHost +// returns shimAddrs and Dial serves a single DNS response, written by +// serveResp, over an in-memory pipe so the fallback path can be exercised +// without a real DNS server. +type fallbackNet struct { + shimAddrs []string + serveResp func(question *dns.Msg) *dns.Msg +} + +// Dial returns one end of an in-memory pipe and spins up a goroutine acting as +// the DNS server on the other end, which reads the SRV query and writes back +// the response produced by serveResp. +func (n *fallbackNet) Dial(_, _ string, + _ time.Duration) (net.Conn, error) { + + client, server := net.Pipe() + + // Act as the DNS server on the far end of the pipe: read the SRV + // query, then write back the crafted response. + go func() { + srvConn := &dns.Conn{Conn: server} + defer srvConn.Close() + + query, err := srvConn.ReadMsg() + if err != nil { + return + } + + _ = srvConn.WriteMsg(n.serveResp(query)) + }() + + return client, nil +} + +// LookupHost returns the configured shim addresses used to reach the fallback +// DNS server. +func (n *fallbackNet) LookupHost(_ string) ([]string, error) { + return n.shimAddrs, nil +} + +// LookupSRV is unsupported by this stub; the fallback path under test issues +// the SRV query manually over the Dial connection instead. +func (n *fallbackNet) LookupSRV(_, _, _ string, + _ time.Duration) (string, []*net.SRV, error) { + + return "", nil, fmt.Errorf("unsupported") +} + +// ResolveTCPAddr is unsupported by this stub as it is not exercised by the +// fallback SRV lookup path. +func (n *fallbackNet) ResolveTCPAddr(_, _ string) (*net.TCPAddr, error) { + return nil, fmt.Errorf("unsupported") +} + // TestGraphBootstrapperSkipsV2Onion ensures SampleNodeAddrs strips Tor v2 // .onion entries from the returned candidate set so the connection manager // never attempts to dial an obsolete v2 hidden service surfaced through the @@ -145,3 +202,163 @@ func TestGraphBootstrapperSkipsV2Onion(t *testing.T) { "v2-only node %x should contribute nothing", s.pub) } } + +// TestFallBackSRVLookupSkipsNonSRV ensures a DNS response whose Answer section +// contains non-SRV records (which an on-path attacker or malicious seed can +// inject, since the response is unauthenticated) is filtered rather than +// triggering a type-assertion panic that would crash the daemon. +func TestFallBackSRVLookupSkipsNonSRV(t *testing.T) { + t.Parallel() + + const target = "nodes.lightning.directory" + + srvTarget := "ln1qexample._nodes._tcp." + target + "." + + netStub := &fallbackNet{ + shimAddrs: []string{"127.0.0.1"}, + serveResp: func(q *dns.Msg) *dns.Msg { + resp := new(dns.Msg) + resp.SetReply(q) + resp.Rcode = dns.RcodeSuccess + + // A hostile/malformed Answer section: an A record and a + // CNAME interleaved with a single valid SRV record. + resp.Answer = []dns.RR{ + &dns.A{ + Hdr: dns.RR_Header{ + Name: q.Question[0].Name, + Rrtype: dns.TypeA, + }, + A: net.ParseIP("1.2.3.4"), + }, + &dns.CNAME{ + Hdr: dns.RR_Header{ + Name: q.Question[0].Name, + Rrtype: dns.TypeCNAME, + }, + Target: "evil.example.", + }, + &dns.SRV{ + Hdr: dns.RR_Header{ + Name: q.Question[0].Name, + Rrtype: dns.TypeSRV, + }, + Target: srvTarget, + Port: 9735, + }, + } + + return resp + }, + } + + bs := NewDNSSeedBootstrapper( + [][2]string{{target, "soa.lightning.directory"}}, + netStub, time.Second, + ) + d, ok := bs.(*DNSSeedBootstrapper) + require.True(t, ok) + + // The non-SRV records must be skipped, leaving only the valid SRV + // record. Crucially, this must not panic. + srvs, err := d.fallBackSRVLookup("soa.lightning.directory", target) + require.NoError(t, err) + require.Len(t, srvs, 1) + require.Equal(t, srvTarget, srvs[0].Target) +} + +// TestFallBackSRVLookupNoSRVRecords ensures a successful DNS response whose +// Answer section holds no SRV records (only CNAME/A entries, or is empty) +// returns an error rather than (nil, nil), so the caller does not mistake "no +// usable records" for a successful query. +func TestFallBackSRVLookupNoSRVRecords(t *testing.T) { + t.Parallel() + + const target = "nodes.lightning.directory" + + netStub := &fallbackNet{ + shimAddrs: []string{"127.0.0.1"}, + serveResp: func(q *dns.Msg) *dns.Msg { + resp := new(dns.Msg) + resp.SetReply(q) + resp.Rcode = dns.RcodeSuccess + + // Only a non-SRV record is present in the Answer + // section, leaving zero usable SRV targets. + resp.Answer = []dns.RR{ + &dns.A{ + Hdr: dns.RR_Header{ + Name: q.Question[0].Name, + Rrtype: dns.TypeA, + }, + A: net.ParseIP("1.2.3.4"), + }, + } + + return resp + }, + } + + bs := NewDNSSeedBootstrapper( + [][2]string{{target, "soa.lightning.directory"}}, + netStub, time.Second, + ) + d, ok := bs.(*DNSSeedBootstrapper) + require.True(t, ok) + + srvs, err := d.fallBackSRVLookup("soa.lightning.directory", target) + require.Error(t, err) + require.Empty(t, srvs) +} + +// TestFallBackSRVLookupEmptyAnswer ensures a successful DNS response with an +// entirely empty Answer section returns an error rather than (nil, nil), so the +// caller does not mistake an empty response for a successful query. +func TestFallBackSRVLookupEmptyAnswer(t *testing.T) { + t.Parallel() + + const target = "nodes.lightning.directory" + + netStub := &fallbackNet{ + shimAddrs: []string{"127.0.0.1"}, + serveResp: func(q *dns.Msg) *dns.Msg { + resp := new(dns.Msg) + resp.SetReply(q) + resp.Rcode = dns.RcodeSuccess + + // Leave the Answer section empty. + resp.Answer = nil + + return resp + }, + } + + bs := NewDNSSeedBootstrapper( + [][2]string{{target, "soa.lightning.directory"}}, + netStub, time.Second, + ) + d, ok := bs.(*DNSSeedBootstrapper) + require.True(t, ok) + + srvs, err := d.fallBackSRVLookup("soa.lightning.directory", target) + require.Error(t, err) + require.Empty(t, srvs) +} + +// TestFallBackSRVLookupNoShimAddrs ensures an empty LookupHost result for the +// fallback shim returns an error instead of panicking on an out-of-bounds +// index. +func TestFallBackSRVLookupNoShimAddrs(t *testing.T) { + t.Parallel() + + netStub := &fallbackNet{shimAddrs: nil} + + bs := NewDNSSeedBootstrapper(nil, netStub, time.Second) + d, ok := bs.(*DNSSeedBootstrapper) + require.True(t, ok) + + _, err := d.fallBackSRVLookup( + "soa.lightning.directory", "nodes.lightning.directory", + ) + require.Error(t, err) +} From edd8e51f919dbf9cf8c29dcd64fc8b5769f8ed8a Mon Sep 17 00:00:00 2001 From: Erick Cestari Date: Tue, 23 Jun 2026 14:52:25 -0300 Subject: [PATCH 2/2] docs: add release note for DNS fallback SRV lookup panic fix modified: to update the release notes at 0.21.1 --- docs/release-notes/release-notes-0.21.1.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/release-notes/release-notes-0.21.1.md b/docs/release-notes/release-notes-0.21.1.md index 207f8f8a2c8..e812f81b3dd 100644 --- a/docs/release-notes/release-notes-0.21.1.md +++ b/docs/release-notes/release-notes-0.21.1.md @@ -29,6 +29,12 @@ which could default new onion service creation to the retired v2 `NEW:RSA1024` key type that modern Tor rejects with `513 Invalid key type`. +* [Fixed a panic](https://github.com/lightningnetwork/lnd/pull/10914) in the + DNS fallback SRV lookup, which unconditionally type-asserted each DNS Answer + record to `*dns.SRV` and crashed the daemon when the response contained a + non-SRV record. Non-SRV records are now skipped, and an empty `LookupHost` + result for the shim no longer triggers an out-of-bounds index. + # New Features ## Functional Enhancements @@ -71,4 +77,5 @@ # Contributors (Alphabetical Order) +* Erick Cestari * Ziggie