From ca9aca9b19b82d2976099c92d9ec7d9de8eb44c4 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Thu, 6 Feb 2025 10:20:31 +0100 Subject: [PATCH] Fix nil pointer exception when load empty list and try to cast it (#3282) --- client/internal/engine.go | 3 +-- client/internal/peer/ice/StunTurn.go | 22 ++++++++++++++++++++++ client/internal/peer/ice/StunTurn_test.go | 13 +++++++++++++ client/internal/peer/ice/agent.go | 3 +-- client/internal/peer/ice/config.go | 4 +--- 5 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 client/internal/peer/ice/StunTurn.go create mode 100644 client/internal/peer/ice/StunTurn_test.go diff --git a/client/internal/engine.go b/client/internal/engine.go index 7f7cdf376..335729d92 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -13,7 +13,6 @@ import ( "sort" "strings" "sync" - "sync/atomic" "time" "github.com/hashicorp/go-multierror" @@ -146,7 +145,7 @@ type Engine struct { STUNs []*stun.URI // TURNs is a list of STUN servers used by ICE TURNs []*stun.URI - stunTurn atomic.Value + stunTurn icemaker.StunTurn clientCtx context.Context clientCancel context.CancelFunc diff --git a/client/internal/peer/ice/StunTurn.go b/client/internal/peer/ice/StunTurn.go new file mode 100644 index 000000000..63ee8c713 --- /dev/null +++ b/client/internal/peer/ice/StunTurn.go @@ -0,0 +1,22 @@ +package ice + +import ( + "sync/atomic" + + "github.com/pion/stun/v2" +) + +type StunTurn atomic.Value + +func (s *StunTurn) Load() []*stun.URI { + v := (*atomic.Value)(s).Load() + if v == nil { + return nil + } + + return v.([]*stun.URI) +} + +func (s *StunTurn) Store(value []*stun.URI) { + (*atomic.Value)(s).Store(value) +} diff --git a/client/internal/peer/ice/StunTurn_test.go b/client/internal/peer/ice/StunTurn_test.go new file mode 100644 index 000000000..7233afa6c --- /dev/null +++ b/client/internal/peer/ice/StunTurn_test.go @@ -0,0 +1,13 @@ +package ice + +import ( + "testing" +) + +func TestStunTurn_LoadEmpty(t *testing.T) { + var stStunTurn StunTurn + got := stStunTurn.Load() + if len(got) != 0 { + t.Errorf("StunTurn.Load() = %v, want %v", got, nil) + } +} diff --git a/client/internal/peer/ice/agent.go b/client/internal/peer/ice/agent.go index dc4750f24..af9e60f2d 100644 --- a/client/internal/peer/ice/agent.go +++ b/client/internal/peer/ice/agent.go @@ -5,7 +5,6 @@ import ( "github.com/pion/ice/v3" "github.com/pion/randutil" - "github.com/pion/stun/v2" log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/client/internal/stdnet" @@ -39,7 +38,7 @@ func NewAgent(iFaceDiscover stdnet.ExternalIFaceDiscover, config Config, candida agentConfig := &ice.AgentConfig{ MulticastDNSMode: ice.MulticastDNSModeDisabled, NetworkTypes: []ice.NetworkType{ice.NetworkTypeUDP4, ice.NetworkTypeUDP6}, - Urls: config.StunTurn.Load().([]*stun.URI), + Urls: config.StunTurn.Load(), CandidateTypes: candidateTypes, InterfaceFilter: stdnet.InterfaceFilter(config.InterfaceBlackList), UDPMux: config.UDPMux, diff --git a/client/internal/peer/ice/config.go b/client/internal/peer/ice/config.go index 8abc842f0..dd854a605 100644 --- a/client/internal/peer/ice/config.go +++ b/client/internal/peer/ice/config.go @@ -1,14 +1,12 @@ package ice import ( - "sync/atomic" - "github.com/pion/ice/v3" ) type Config struct { // StunTurn is a list of STUN and TURN URLs - StunTurn *atomic.Value // []*stun.URI + StunTurn *StunTurn // []*stun.URI // InterfaceBlackList is a list of machine interfaces that should be filtered out by ICE Candidate gathering // (e.g. if eth0 is in the list, host candidate of this interface won't be used)