From 6f605b2de2159359e38c866c0277b2ac86b4ec36 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 17 Jan 2019 14:11:56 +0100 Subject: [PATCH 1/5] channeldb/addr_test: inspect error string Lets us match on more than static errors. --- channeldb/addr_test.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/channeldb/addr_test.go b/channeldb/addr_test.go index c4bc4e8ee..c3d4e07f1 100644 --- a/channeldb/addr_test.go +++ b/channeldb/addr_test.go @@ -3,6 +3,7 @@ package channeldb import ( "bytes" "net" + "strings" "testing" "github.com/lightningnetwork/lnd/tor" @@ -15,7 +16,7 @@ func (t unknownAddrType) String() string { return "unknown" } var addrTests = []struct { expAddr net.Addr - serErr error + serErr string }{ { expAddr: &net.TCPAddr{ @@ -43,7 +44,7 @@ var addrTests = []struct { }, { expAddr: unknownAddrType{}, - serErr: ErrUnknownAddressType, + serErr: ErrUnknownAddressType.Error(), }, } @@ -55,11 +56,21 @@ func TestAddrSerialization(t *testing.T) { var b bytes.Buffer for _, test := range addrTests { err := serializeAddr(&b, test.expAddr) - if err != test.serErr { + switch { + case err == nil && test.serErr != "": + t.Fatalf("expected serialization err for addr %v", + test.expAddr) + + case err != nil && test.serErr == "": + t.Fatalf("unexpected serialization err for addr %v: %v", + test.expAddr, err) + + case err != nil && !strings.Contains(err.Error(), test.serErr): t.Fatalf("unexpected serialization err for addr %v, "+ - "want: %v, got %v", - test.expAddr, test.serErr, err) - } else if test.serErr != nil { + "want: %v, got %v", test.expAddr, test.serErr, + err) + + case err != nil: continue } From 02b7bb356ae43e1976796c79e151b3054c1b5174 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 17 Jan 2019 14:11:56 +0100 Subject: [PATCH 2/5] channeldb/addr: validate ip before writing --- channeldb/addr.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/channeldb/addr.go b/channeldb/addr.go index 0d40099e6..18e8bba1f 100644 --- a/channeldb/addr.go +++ b/channeldb/addr.go @@ -3,6 +3,7 @@ package channeldb import ( "encoding/binary" "errors" + "fmt" "io" "net" @@ -43,6 +44,10 @@ func encodeTCPAddr(w io.Writer, addr *net.TCPAddr) error { ip = addr.IP.To16() } + if ip == nil { + return fmt.Errorf("unable to encode IP %v", addr.IP) + } + if _, err := w.Write([]byte{addrType}); err != nil { return err } From e2e00f9dd2d6f1b86e56efc85b9c1eabf3aa6bcf Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 17 Jan 2019 14:11:56 +0100 Subject: [PATCH 3/5] channeldb/addr_test: add invalid TCP cases These are invalid length IP addresses that earlier would not fail to serialize. --- channeldb/addr_test.go | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/channeldb/addr_test.go b/channeldb/addr_test.go index c3d4e07f1..ad0da59e5 100644 --- a/channeldb/addr_test.go +++ b/channeldb/addr_test.go @@ -14,19 +14,23 @@ type unknownAddrType struct{} func (t unknownAddrType) Network() string { return "unknown" } func (t unknownAddrType) String() string { return "unknown" } +var testIP4 = net.ParseIP("192.168.1.1") +var testIP6 = net.ParseIP("2001:0db8:0000:0000:0000:ff00:0042:8329") + var addrTests = []struct { expAddr net.Addr serErr string }{ + // Valid addresses. { expAddr: &net.TCPAddr{ - IP: net.ParseIP("192.168.1.1"), + IP: testIP4, Port: 12345, }, }, { expAddr: &net.TCPAddr{ - IP: net.ParseIP("2001:0db8:0000:0000:0000:ff00:0042:8329"), + IP: testIP6, Port: 65535, }, }, @@ -42,10 +46,44 @@ var addrTests = []struct { Port: 80, }, }, + + // Invalid addresses. { expAddr: unknownAddrType{}, serErr: ErrUnknownAddressType.Error(), }, + { + expAddr: &net.TCPAddr{ + // Remove last byte of IPv4 address. + IP: testIP4[:len(testIP4)-1], + Port: 12345, + }, + serErr: "unable to encode", + }, + { + expAddr: &net.TCPAddr{ + // Add an extra byte of IPv4 address. + IP: append(testIP4, 0xff), + Port: 12345, + }, + serErr: "unable to encode", + }, + { + expAddr: &net.TCPAddr{ + // Remove last byte of IPv6 address. + IP: testIP6[:len(testIP6)-1], + Port: 65535, + }, + serErr: "unable to encode", + }, + { + expAddr: &net.TCPAddr{ + // Add an extra byte to the IPv6 address. + IP: append(testIP6, 0xff), + Port: 65535, + }, + serErr: "unable to encode", + }, } // TestAddrSerialization tests that the serialization method used by channeldb From 6a3e1423d269c075ac6c21fda430c0cdfad4772e Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 17 Jan 2019 14:11:56 +0100 Subject: [PATCH 4/5] channeldb/addr: sanity check onion address length before writing to db --- channeldb/addr.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/channeldb/addr.go b/channeldb/addr.go index 18e8bba1f..dd057265c 100644 --- a/channeldb/addr.go +++ b/channeldb/addr.go @@ -69,7 +69,8 @@ func encodeTCPAddr(w io.Writer, addr *net.TCPAddr) error { // representation. func encodeOnionAddr(w io.Writer, addr *tor.OnionAddr) error { var suffixIndex int - switch len(addr.OnionService) { + hostLen := len(addr.OnionService) + switch hostLen { case tor.V2Len: if _, err := w.Write([]byte{byte(v2OnionAddr)}); err != nil { return err @@ -84,12 +85,29 @@ func encodeOnionAddr(w io.Writer, addr *tor.OnionAddr) error { return errors.New("unknown onion service length") } + suffix := addr.OnionService[suffixIndex:] + if suffix != tor.OnionSuffix { + return fmt.Errorf("invalid suffix \"%v\"", suffix) + } + host, err := tor.Base32Encoding.DecodeString( addr.OnionService[:suffixIndex], ) if err != nil { return err } + + // Sanity check the decoded length. + switch { + case hostLen == tor.V2Len && len(host) != tor.V2DecodedLen: + return fmt.Errorf("onion service %v decoded to invalid host %x", + addr.OnionService, host) + + case hostLen == tor.V3Len && len(host) != tor.V3DecodedLen: + return fmt.Errorf("onion service %v decoded to invalid host %x", + addr.OnionService, host) + } + if _, err := w.Write(host); err != nil { return err } From 8af247364452ed65d6a8f76894f17ee4b00372af Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 17 Jan 2019 14:11:56 +0100 Subject: [PATCH 5/5] channeldb/addr_test: add tests for invalid onion addresses --- channeldb/addr_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/channeldb/addr_test.go b/channeldb/addr_test.go index ad0da59e5..c761989c0 100644 --- a/channeldb/addr_test.go +++ b/channeldb/addr_test.go @@ -84,6 +84,30 @@ var addrTests = []struct { }, serErr: "unable to encode", }, + { + expAddr: &tor.OnionAddr{ + // Invalid suffix. + OnionService: "vww6ybal4bd7szmgncyruucpgfkqahzddi37ktceo3ah7ngmcopnpyyd.inion", + Port: 80, + }, + serErr: "invalid suffix", + }, + { + expAddr: &tor.OnionAddr{ + // Invalid length. + OnionService: "vww6ybal4bd7szmgncyruucpgfkqahzddi37ktceo3ah7ngmcopnpyy.onion", + Port: 80, + }, + serErr: "unknown onion service length", + }, + { + expAddr: &tor.OnionAddr{ + // Invalid encoding. + OnionService: "vww6ybal4bd7szmgncyruucpgfkqahzddi37ktceo3ah7ngmcopnpyyA.onion", + Port: 80, + }, + serErr: "illegal base32", + }, } // TestAddrSerialization tests that the serialization method used by channeldb