diff --git a/contrib/mobile/mobile.go b/contrib/mobile/mobile.go index 72ea7d68..c7213707 100644 --- a/contrib/mobile/mobile.go +++ b/contrib/mobile/mobile.go @@ -138,7 +138,7 @@ func (m *Yggdrasil) SendBuffer(p []byte, length int) error { if m.iprwc == nil { return nil } - if len(p) < length { + if length < 0 || len(p) < length { return nil } _, _ = m.iprwc.Write(p[:length]) @@ -231,8 +231,7 @@ func (m *Yggdrasil) GetPeersJSON() (result string) { }{} for _, v := range m.core.GetPeers() { var ip string - if v.Key != nil { - a := address.AddrForKey(v.Key) + if a := address.AddrForKey(v.Key); a != nil { ip = net.IP(a[:]).String() } peers = append(peers, struct { @@ -286,11 +285,18 @@ func SummaryForConfig(b []byte) *ConfigSummary { if err := cfg.UnmarshalHJSON(b); err != nil { return nil } + if len(cfg.PrivateKey) != ed25519.PrivateKeySize { + return nil + } pub := ed25519.PrivateKey(cfg.PrivateKey).Public().(ed25519.PublicKey) hpub := hex.EncodeToString(pub) - addr := net.IP(address.AddrForKey(pub)[:]) + addrPtr, snetPtr := address.AddrForKey(pub), address.SubnetForKey(pub) + if addrPtr == nil || snetPtr == nil { + return nil + } + addr := net.IP(addrPtr[:]) snet := net.IPNet{ - IP: append(address.SubnetForKey(pub)[:], 0, 0, 0, 0, 0, 0, 0, 0), + IP: append(snetPtr[:], 0, 0, 0, 0, 0, 0, 0, 0), Mask: net.CIDRMask(64, 128), } return &ConfigSummary{ diff --git a/contrib/mobile/mobile_test.go b/contrib/mobile/mobile_test.go index 74689294..856f890d 100644 --- a/contrib/mobile/mobile_test.go +++ b/contrib/mobile/mobile_test.go @@ -26,3 +26,29 @@ func TestStartYggdrasil(t *testing.T) { t.Fatalf("Failed to stop Yggdrasil: %s", err) } } + +// SendBuffer previously panicked when the caller passed a negative +// length (p[:length] out of range) and Send/SendBuffer with empty +// payload reached writePC, which also panicked. +func TestSendBufferRejectsBadLength(t *testing.T) { + logger := log.New(os.Stdout, "", 0) + ygg := &Yggdrasil{logger: logger} + if err := ygg.StartAutoconfigure(); err != nil { + t.Fatalf("Failed to start Yggdrasil: %s", err) + } + defer func() { _ = ygg.Stop() }() + defer func() { + if r := recover(); r != nil { + t.Fatalf("SendBuffer must not panic on bad length, got: %v", r) + } + }() + if err := ygg.SendBuffer([]byte{1, 2, 3, 4}, -1); err != nil { + t.Fatalf("SendBuffer returned unexpected error: %s", err) + } + if err := ygg.SendBuffer(nil, 0); err != nil { + t.Fatalf("SendBuffer returned unexpected error: %s", err) + } + if err := ygg.Send(nil); err != nil { + t.Fatalf("Send returned unexpected error: %s", err) + } +} diff --git a/src/admin/admin.go b/src/admin/admin.go index 305cac30..b7ad15ef 100644 --- a/src/admin/admin.go +++ b/src/admin/admin.go @@ -91,7 +91,15 @@ func New(c *core.Core, log core.Logger, opts ...SetupOption) (*AdminSocket, erro case "unix": if _, err := os.Stat(u.Path); err == nil { a.log.Debugln("Admin socket", u.Path, "already exists, trying to clean up") - if _, err := net.DialTimeout("unix", u.Path, time.Second*2); err == nil || err.(net.Error).Timeout() { + _, dialErr := net.DialTimeout("unix", u.Path, time.Second*2) + inUse := dialErr == nil + if !inUse { + var netErr net.Error + if errors.As(dialErr, &netErr) && netErr.Timeout() { + inUse = true + } + } + if inUse { a.log.Errorln("Admin socket", u.Path, "already exists and is in use by another process") os.Exit(1) } else { @@ -105,9 +113,8 @@ func New(c *core.Core, log core.Logger, opts ...SetupOption) (*AdminSocket, erro } a.listener, err = net.Listen("unix", u.Path) if err == nil { - switch u.Path[:1] { - case "@": // maybe abstract namespace - default: + abstract := u.Path != "" && u.Path[0] == '@' + if !abstract { if err := os.Chmod(u.Path, 0660); err != nil { a.log.Warnln("WARNING:", u.Path, "may have unsafe permissions!") } diff --git a/src/admin/getpaths.go b/src/admin/getpaths.go index 250f4c6a..08b2bc5d 100644 --- a/src/admin/getpaths.go +++ b/src/admin/getpaths.go @@ -28,6 +28,9 @@ func (a *AdminSocket) getPathsHandler(_ *GetPathsRequest, res *GetPathsResponse) res.Paths = make([]PathEntry, 0, len(paths)) for _, p := range paths { addr := address.AddrForKey(p.Key) + if addr == nil { + continue + } res.Paths = append(res.Paths, PathEntry{ IPAddress: net.IP(addr[:]).String(), PublicKey: hex.EncodeToString(p.Key), diff --git a/src/admin/getsessions.go b/src/admin/getsessions.go index 2d76a35b..4a917011 100644 --- a/src/admin/getsessions.go +++ b/src/admin/getsessions.go @@ -28,6 +28,9 @@ func (a *AdminSocket) getSessionsHandler(_ *GetSessionsRequest, res *GetSessions res.Sessions = make([]SessionEntry, 0, len(sessions)) for _, s := range sessions { addr := address.AddrForKey(s.Key) + if addr == nil { + continue + } res.Sessions = append(res.Sessions, SessionEntry{ IPAddress: net.IP(addr[:]).String(), PublicKey: hex.EncodeToString(s.Key[:]), diff --git a/src/admin/gettree.go b/src/admin/gettree.go index 993827d9..00212dcd 100644 --- a/src/admin/gettree.go +++ b/src/admin/gettree.go @@ -27,6 +27,9 @@ func (a *AdminSocket) getTreeHandler(_ *GetTreeRequest, res *GetTreeResponse) er res.Tree = make([]TreeEntry, 0, len(tree)) for _, d := range tree { addr := address.AddrForKey(d.Key) + if addr == nil { + continue + } res.Tree = append(res.Tree, TreeEntry{ IPAddress: net.IP(addr[:]).String(), PublicKey: hex.EncodeToString(d.Key[:]), diff --git a/src/config/config.go b/src/config/config.go index 5dd7b3d4..1d01ed58 100644 --- a/src/config/config.go +++ b/src/config/config.go @@ -99,8 +99,8 @@ func (cfg *NodeConfig) ReadFrom(r io.Reader) (int64, error) { // throwing everywhere when it's converting things into UTF-16 for the hell // of it - remove it and decode back down into UTF-8. This is necessary // because hjson doesn't know what to do with UTF-16 and will panic - if bytes.Equal(conf[0:2], []byte{0xFF, 0xFE}) || - bytes.Equal(conf[0:2], []byte{0xFE, 0xFF}) { + if len(conf) >= 2 && (bytes.Equal(conf[0:2], []byte{0xFF, 0xFE}) || + bytes.Equal(conf[0:2], []byte{0xFE, 0xFF})) { utf := unicode.UTF16(unicode.BigEndian, unicode.UseBOM) decoder := utf.NewDecoder() conf, err = decoder.Bytes(conf) diff --git a/src/config/config_test.go b/src/config/config_test.go index 6b74b50f..c8e311f7 100644 --- a/src/config/config_test.go +++ b/src/config/config_test.go @@ -1,9 +1,33 @@ package config import ( + "bytes" "testing" ) +// ReadFrom previously sliced conf[0:2] for the BOM check without +// guarding the length, so empty or single-byte configs piped via +// -useconf panicked with index out of range. +func TestConfigReadFromEmpty(t *testing.T) { + for _, tc := range []struct { + name string + body []byte + }{ + {name: "empty", body: nil}, + {name: "single byte", body: []byte{'{'}}, + } { + t.Run(tc.name, func(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("ReadFrom must not panic on short input, got: %v", r) + } + }() + var cfg NodeConfig + _, _ = cfg.ReadFrom(bytes.NewReader(tc.body)) + }) + } +} + func TestConfig_Keys(t *testing.T) { /* var nodeConfig NodeConfig diff --git a/src/core/nodeinfo.go b/src/core/nodeinfo.go index 698aac7f..3e026e1d 100644 --- a/src/core/nodeinfo.go +++ b/src/core/nodeinfo.go @@ -1,6 +1,7 @@ package core import ( + "crypto/ed25519" "encoding/hex" "encoding/json" "errors" @@ -151,6 +152,9 @@ func (m *nodeinfo) nodeInfoAdminHandler(in json.RawMessage) (interface{}, error) if kbs, err = hex.DecodeString(req.Key); err != nil { return nil, fmt.Errorf("failed to decode public key: %w", err) } + if len(kbs) != ed25519.PublicKeySize { + return nil, fmt.Errorf("invalid public key length") + } copy(key[:], kbs) ch := make(chan []byte, 1) m.sendReq(nil, key, func(info json.RawMessage) { diff --git a/src/ipv6rwc/ipv6rwc.go b/src/ipv6rwc/ipv6rwc.go index 59f4f022..c1ae2eae 100644 --- a/src/ipv6rwc/ipv6rwc.go +++ b/src/ipv6rwc/ipv6rwc.go @@ -281,6 +281,9 @@ func (k *keyStore) readPC(p []byte) (int, error) { } func (k *keyStore) writePC(bs []byte) (int, error) { + if len(bs) == 0 { + return 0, errors.New("empty packet") + } if bs[0]&0xf0 != 0x60 { return 0, errors.New("not an IPv6 packet") // not IPv6 } diff --git a/src/ipv6rwc/ipv6rwc_test.go b/src/ipv6rwc/ipv6rwc_test.go new file mode 100644 index 00000000..75d72900 --- /dev/null +++ b/src/ipv6rwc/ipv6rwc_test.go @@ -0,0 +1,70 @@ +package ipv6rwc + +import ( + "testing" +) + +// writePC indexed bs[0] before checking len(bs), so an empty buffer +// reaching it (via the mobile Send/SendBuffer API or a zero-byte TUN +// read) panicked with index out of range. Reject empty input cleanly. +func TestWritePCRejectsEmptyPacket(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("writePC must not panic on empty input, got: %v", r) + } + }() + var k keyStore + if _, err := k.writePC(nil); err == nil { + t.Fatalf("expected an error for nil input, got nil") + } + if _, err := k.writePC([]byte{}); err == nil { + t.Fatalf("expected an error for zero-length input, got nil") + } +} + +// Buffers shorter than the minimum IPv6 header (40 bytes) but non-empty +// must still be rejected without panicking. +func TestWritePCRejectsTruncatedPacket(t *testing.T) { + var k keyStore + for _, tc := range []struct { + name string + buf []byte + }{ + {name: "single byte ipv6 marker", buf: []byte{0x60}}, + {name: "ten bytes ipv6 marker", buf: append([]byte{0x60}, make([]byte, 9)...)}, + {name: "thirty-nine bytes ipv6 marker", buf: append([]byte{0x60}, make([]byte, 38)...)}, + } { + t.Run(tc.name, func(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("writePC must not panic on truncated input, got: %v", r) + } + }() + if _, err := k.writePC(tc.buf); err == nil { + t.Fatalf("expected an error for truncated input, got nil") + } + }) + } +} + +// Non-IPv6 packets (e.g. IPv4) must report "not an IPv6 packet" rather +// than the length-based error, regardless of whether they meet the IPv6 +// 40-byte minimum. +func TestWritePCReportsIPv4AsNonIPv6(t *testing.T) { + var k keyStore + for _, tc := range []struct { + name string + buf []byte + }{ + {name: "ipv4 short", buf: []byte{0x45, 0, 0, 20}}, + {name: "ipv4 padded to 40", buf: append([]byte{0x45, 0, 0, 60}, make([]byte, 36)...)}, + {name: "ipv4 padded to 64", buf: append([]byte{0x45, 0, 0, 60}, make([]byte, 60)...)}, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := k.writePC(tc.buf) + if err == nil || err.Error() != "not an IPv6 packet" { + t.Fatalf("expected \"not an IPv6 packet\" error, got: %v", err) + } + }) + } +} diff --git a/src/multicast/multicast.go b/src/multicast/multicast.go index 49d4af23..8c169108 100644 --- a/src/multicast/multicast.go +++ b/src/multicast/multicast.go @@ -392,7 +392,8 @@ func (m *Multicast) listen() { if !m.IsStarted() { return } - panic(err) + m.log.Warnln("Multicast listener read error:", err) + continue } if rcm != nil { // Windows can't set the flag needed to return a non-nil value here @@ -417,7 +418,10 @@ func (m *Multicast) listen() { case adv.PublicKey.Equal(m.core.PublicKey()): continue } - from := fromAddr.(*net.UDPAddr) + from, ok := fromAddr.(*net.UDPAddr) + if !ok { + continue + } from.Port = int(adv.Port) var interfaces map[string]*interfaceInfo phony.Block(m, func() {