lnwire: fix NodeAnnouncement2 address decoding

docs/release-notes/release-notes-0.22.0.md

@@ -58,6 +58,10 @@
   no longer hangs on a fresh database when `db.postgres.maxconnections` is
   set to a low value.
 
+* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/11219) in the
+  `NodeAnnouncement2` address decoders, where all decoded addresses aliased
+  one scratch array and truncated records decoded without an error.
+
 # New Features
 
 ## Functional Enhancements
@@ -201,3 +205,4 @@
 * Jared Tobin
 * Kevin Cai
 * Vandit Singh
+* s1ns3nz0

lnwire/node_announcement_2.go

@@ -395,6 +395,18 @@ func ipv4AddrsEncoder(w io.Writer, val interface{}, _ *[8]byte) error {
 	return tlv.NewTypeForEncodingErr(val, "lnwire.IPV4Addrs")
 }
 
+// readAddrField fills b from an address list record. The TLV length declares
+// every byte of the list, so an io.EOF at an address boundary is a truncation
+// and returns io.ErrUnexpectedEOF.
+func readAddrField(r io.Reader, b []byte) error {
+	_, err := io.ReadFull(r, b)
+	if errors.Is(err, io.EOF) {
+		return io.ErrUnexpectedEOF
+	}
+
+	return err
+}
+
 // ipv4AddrsDecoder decodes TLV bytes into IPv4 addresses.
 func ipv4AddrsDecoder(r io.Reader, val interface{}, _ *[8]byte,
 	l uint64) error {
@@ -406,15 +418,19 @@ func ipv4AddrsDecoder(r io.Reader, val interface{}, _ *[8]byte,
 		var (
 			numAddrs = int(l / ipv4AddrEncodedSize)
 			addrs    = make([]*net.TCPAddr, 0, numAddrs)
-			ip       [4]byte
-			port     [2]byte
 		)
 		for len(addrs) < numAddrs {
-			_, err := r.Read(ip[:])
+			// Each address owns its arrays, so no two share bytes.
+			var (
+				ip   [4]byte
+				port [2]byte
+			)
+
+			err := readAddrField(r, ip[:])
 			if err != nil {
 				return err
 			}
-			_, err = r.Read(port[:])
+			err = readAddrField(r, port[:])
 			if err != nil {
 				return err
 			}
@@ -486,15 +502,19 @@ func ipv6AddrsDecoder(r io.Reader, val interface{}, _ *[8]byte,
 		var (
 			numAddrs = int(l / ipv6AddrEncodedSize)
 			addrs    = make([]*net.TCPAddr, 0, numAddrs)
-			ip       [16]byte
-			port     [2]byte
 		)
 		for len(addrs) < numAddrs {
-			_, err := r.Read(ip[:])
+			// Each address owns its arrays, so no two share bytes.
+			var (
+				ip   [16]byte
+				port [2]byte
+			)
+
+			err := readAddrField(r, ip[:])
 			if err != nil {
 				return err
 			}
-			_, err = r.Read(port[:])
+			err = readAddrField(r, port[:])
 			if err != nil {
 				return err
 			}
@@ -578,19 +598,25 @@ func torV3AddrsDecoder(r io.Reader, val interface{}, _ *[8]byte,
 		var (
 			numAddrs = int(l / torV3AddrEncodedSize)
 			addrs    = make([]*tor.OnionAddr, 0, numAddrs)
-			ip       [tor.V3DecodedLen]byte
-			p        [2]byte
 		)
 		for len(addrs) < numAddrs {
-			_, err := r.Read(ip[:])
+			// Each address owns its arrays, so no two share bytes.
+			var (
+				host [tor.V3DecodedLen]byte
+				port [2]byte
+			)
+
+			err := readAddrField(r, host[:])
 			if err != nil {
 				return err
 			}
-			_, err = r.Read(p[:])
+			err = readAddrField(r, port[:])
 			if err != nil {
 				return err
 			}
-			onionService := tor.Base32Encoding.EncodeToString(ip[:])
+			onionService := tor.Base32Encoding.EncodeToString(
+				host[:],
+			)
 			onionService += tor.OnionSuffix
 
 			if len(onionService) != tor.V3Len {
@@ -599,10 +625,10 @@ func torV3AddrsDecoder(r io.Reader, val interface{}, _ *[8]byte,
 					tor.V3Len, len(onionService))
 			}
 
-			port := int(binary.BigEndian.Uint16(p[:]))
+			portNum := int(binary.BigEndian.Uint16(port[:]))
 			addrs = append(addrs, &tor.OnionAddr{
 				OnionService: onionService,
-				Port:         port,
+				Port:         portNum,
 			})
 		}
 		*v = addrs

lnwire/node_announcement_2_test.go

@@ -2,6 +2,7 @@ package lnwire
 
 import (
 	"bytes"
+	"io"
 	"testing"
 
 	"github.com/lightningnetwork/lnd/tlv"
@@ -51,14 +52,17 @@ func TestNodeAnn2EncodeDecode(t *testing.T) {
 		0x05,                               // type.
 		0x0c,                               // length (12 bytes).
 		0x0a, 0x00, 0x00, 0x01, 0x23, 0x28, // 10.0.0.1:9000
-		0x0a, 0x00, 0x00, 0x01, 0x1f, 0x90, // 10.0.0.1:8080
+		0x0a, 0x00, 0x00, 0x02, 0x1f, 0x90, // 10.0.0.2:8080
 
 		// IPV6Addrs record (optional).
 		0x07,                                           // type.
-		0x12,                                           // length.
+		0x24,                                           // length.
 		0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, // IPv6 address
 		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, // 2001:db8::1
 		0x1f, 0x40, // port 8000
+		0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, // IPv6 address
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, // 2001:db8::2
+		0x1f, 0x41, // port 8001
 
 		// TorV3Addrs record (optional).
 		0x09, // type.
@@ -117,3 +121,99 @@ func TestNodeAnn2EncodeDecode(t *testing.T) {
 	// bytes.
 	require.Equal(t, rawBytes, b.Bytes())
 }
+
+// TestNodeAnn2AddrDecodeTruncated asserts that each fixed width address decoder
+// rejects a record that holds fewer bytes than its TLV length declares.
+func TestNodeAnn2AddrDecodeTruncated(t *testing.T) {
+	t.Parallel()
+
+	// Each family decodes addresses of one fixed width, so a record that
+	// holds fewer bytes than its length declares must fail instead of
+	// reading the untouched bytes of the decoder's scratch array.
+	families := []struct {
+		name   string
+		size   uint64
+		decode func(io.Reader, uint64) error
+	}{
+		{
+			name: "ipv4",
+			size: ipv4AddrEncodedSize,
+			decode: func(r io.Reader, l uint64) error {
+				var (
+					addrs IPV4Addrs
+					buf   [8]byte
+				)
+
+				return ipv4AddrsDecoder(r, &addrs, &buf, l)
+			},
+		},
+		{
+			name: "ipv6",
+			size: ipv6AddrEncodedSize,
+			decode: func(r io.Reader, l uint64) error {
+				var (
+					addrs IPV6Addrs
+					buf   [8]byte
+				)
+
+				return ipv6AddrsDecoder(r, &addrs, &buf, l)
+			},
+		},
+		{
+			name: "tor v3",
+			size: torV3AddrEncodedSize,
+			decode: func(r io.Reader, l uint64) error {
+				var (
+					addrs TorV3Addrs
+					buf   [8]byte
+				)
+
+				return torV3AddrsDecoder(r, &addrs, &buf, l)
+			},
+		},
+	}
+
+	for _, family := range families {
+		// Each decoder reads the address and the port separately. A
+		// record one byte short truncates only the port, so half a
+		// record is needed to reach the address read as well. A record
+		// that stops between two addresses makes the next read return
+		// no bytes at all.
+		cases := []struct {
+			name     string
+			length   uint64
+			supplied uint64
+		}{
+			{
+				name:     "short address",
+				length:   family.size,
+				supplied: family.size / 2,
+			},
+			{
+				name:     "short port",
+				length:   family.size,
+				supplied: family.size - 1,
+			},
+			{
+				name:     "address boundary",
+				length:   2 * family.size,
+				supplied: family.size,
+			},
+		}
+
+		for _, tc := range cases {
+			name := family.name + "/" + tc.name
+
+			t.Run(name, func(t *testing.T) {
+				t.Parallel()
+
+				err := family.decode(
+					bytes.NewReader(
+						make([]byte, tc.supplied),
+					), tc.length,
+				)
+				require.ErrorIs(t, err, io.ErrUnexpectedEOF)
+			})
+		}
+	}
+}

lnwire/test_message.go

@@ -1441,16 +1441,29 @@ func (n *NodeAnnouncement2) RandTestMessage(t *rapid.T) Message {
 	}
 
 	if rapid.Bool().Draw(t, "includeIPV4Addrs") {
-		ipv4Addrs := make(IPV4Addrs, 1)
-		ip := make(net.IP, 4)
-		ip[0] = uint8(rapid.IntRange(1, 223).Draw(t, "ip4_0"))
-		ip[1] = uint8(rapid.IntRange(0, 255).Draw(t, "ip4_1"))
-		ip[2] = uint8(rapid.IntRange(0, 255).Draw(t, "ip4_2"))
-		ip[3] = uint8(rapid.IntRange(1, 254).Draw(t, "ip4_3"))
-
-		ipv4Addrs[0] = &net.TCPAddr{
-			IP:   ip,
-			Port: rapid.IntRange(1, 65535).Draw(t, "port4"),
+		numIPV4 := rapid.IntRange(1, 3).Draw(t, "numIPV4Addrs")
+		ipv4Addrs := make(IPV4Addrs, numIPV4)
+		for i := range numIPV4 {
+			ip := make(net.IP, 4)
+			ip[0] = uint8(rapid.IntRange(1, 223).Draw(
+				t, fmt.Sprintf("ip4_%d_0", i),
+			))
+			ip[1] = uint8(rapid.IntRange(0, 255).Draw(
+				t, fmt.Sprintf("ip4_%d_1", i),
+			))
+			ip[2] = uint8(rapid.IntRange(0, 255).Draw(
+				t, fmt.Sprintf("ip4_%d_2", i),
+			))
+			ip[3] = uint8(rapid.IntRange(1, 254).Draw(
+				t, fmt.Sprintf("ip4_%d_3", i),
+			))
+
+			ipv4Addrs[i] = &net.TCPAddr{
+				IP: ip,
+				Port: rapid.IntRange(1, 65535).Draw(
+					t, fmt.Sprintf("port4_%d", i),
+				),
+			}
 		}
 
 		ipv4Record := tlv.ZeroRecordT[tlv.TlvType5, IPV4Addrs]()
@@ -1459,18 +1472,22 @@ func (n *NodeAnnouncement2) RandTestMessage(t *rapid.T) Message {
 	}
 
 	if rapid.Bool().Draw(t, "includeIPV6Addrs") {
-		ipv6Addrs := make(IPV6Addrs, 1)
-		ip := make(net.IP, 16)
-		// Generate random IPv6 address.
-		for j := range 16 {
-			ip[j] = uint8(rapid.IntRange(0, 255).Draw(
-				t, fmt.Sprintf("ip6_%d", j)),
-			)
-		}
+		numIPV6 := rapid.IntRange(1, 3).Draw(t, "numIPV6Addrs")
+		ipv6Addrs := make(IPV6Addrs, numIPV6)
+		for i := range numIPV6 {
+			ip := make(net.IP, 16)
+			for j := range 16 {
+				ip[j] = uint8(rapid.IntRange(0, 255).Draw(
+					t, fmt.Sprintf("ip6_%d_%d", i, j),
+				))
+			}
 
-		ipv6Addrs[0] = &net.TCPAddr{
-			IP:   ip,
-			Port: rapid.IntRange(1, 65535).Draw(t, "port6"),
+			ipv6Addrs[i] = &net.TCPAddr{
+				IP: ip,
+				Port: rapid.IntRange(1, 65535).Draw(
+					t, fmt.Sprintf("port6_%d", i),
+				),
+			}
 		}
 
 		ipv6Record := tlv.ZeroRecordT[tlv.TlvType7, IPV6Addrs]()
@@ -1479,18 +1496,22 @@ func (n *NodeAnnouncement2) RandTestMessage(t *rapid.T) Message {
 	}
 
 	if rapid.Bool().Draw(t, "includeTorV3Addrs") {
-		torV3Addrs := make(TorV3Addrs, 1)
-		onionBytes := rapid.SliceOfN(rapid.Byte(), 35, 35).Draw(
-			t, "onion",
-		)
-		onionService := tor.Base32Encoding.EncodeToString(onionBytes) +
-			tor.OnionSuffix
-
-		torV3Addrs[0] = &tor.OnionAddr{
-			OnionService: onionService,
-			Port: rapid.IntRange(1, 65535).Draw(
-				t, "torPort",
-			),
+		numTorV3 := rapid.IntRange(1, 3).Draw(t, "numTorV3Addrs")
+		torV3Addrs := make(TorV3Addrs, numTorV3)
+		for i := range numTorV3 {
+			onionBytes := rapid.SliceOfN(rapid.Byte(), 35, 35).Draw(
+				t, fmt.Sprintf("onion_%d", i),
+			)
+			onionService := tor.Base32Encoding.EncodeToString(
+				onionBytes,
+			) + tor.OnionSuffix
+
+			torV3Addrs[i] = &tor.OnionAddr{
+				OnionService: onionService,
+				Port: rapid.IntRange(1, 65535).Draw(
+					t, fmt.Sprintf("torPort_%d", i),
+				),
+			}
 		}
 
 		torV3Record := tlv.ZeroRecordT[tlv.TlvType9, TorV3Addrs]()