From bcd6915c6722610449de073034407be52f0551ef Mon Sep 17 00:00:00 2001 From: Jo Turk Date: Mon, 13 Jul 2026 07:09:15 +0300 Subject: [PATCH] Ignore malformed nomination attributes --- selection.go | 14 +++++---- selection_test.go | 74 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/selection.go b/selection.go index 6079906a..977df68d 100644 --- a/selection.go +++ b/selection.go @@ -445,16 +445,20 @@ func (s *controlledSelector) HandleBindingRequest(message *stun.Message, local, } pair.UpdateRequestReceived() - if message.Contains(stun.AttrUseCandidate) || message.Contains(s.agent.nominationAttribute) { //nolint:nestif - // https://tools.ietf.org/html/rfc8445#section-7.3.1.5 - - // Check for renomination attribute - var nominationValue *uint32 + hasUseCandidate := message.Contains(stun.AttrUseCandidate) + hasValidNomination := false + var nominationValue *uint32 + if message.Contains(s.agent.nominationAttribute) { var nomination NominationAttribute if err := nomination.GetFromWithType(message, s.agent.nominationAttribute); err == nil { nominationValue = &nomination.Value + hasValidNomination = true s.log.Tracef("Received nomination with value %d", nomination.Value) } + } + + if hasUseCandidate || hasValidNomination { //nolint:nestif + // https://tools.ietf.org/html/rfc8445#section-7.3.1.5 // Check if we should accept this nomination based on renomination rules if !s.shouldAcceptNomination(nominationValue) { diff --git a/selection_test.go b/selection_test.go index a990ed4f..5927e9db 100644 --- a/selection_test.go +++ b/selection_test.go @@ -31,6 +31,12 @@ const ( selectionTestLocalUfrag = "local" ) +type stunSetterFunc func(*stun.Message) error + +func (f stunSetterFunc) AddTo(m *stun.Message) error { + return f(m) +} + func sendUntilDone(t *testing.T, writingConn, readingConn net.Conn, maxAttempts int) bool { t.Helper() @@ -1517,6 +1523,33 @@ func TestLiteControlledSelector_NoPingCandidate(t *testing.T) { return msg } + buildMalformedNominationMsg := func(t *testing.T, agent *Agent, useCandidate bool) *stun.Message { + t.Helper() + + setters := []stun.Setter{ + stun.BindingRequest, + stun.TransactionID, + stun.NewUsername(agent.localUfrag + ":" + agent.remoteUfrag), + } + if useCandidate { + setters = append(setters, UseCandidate()) + } + setters = append(setters, + stunSetterFunc(func(m *stun.Message) error { + m.Add(agent.nominationAttribute, []byte{0x01, 0x02}) + + return nil + }), + stun.NewShortTermIntegrity(agent.localPwd), + stun.Fingerprint, + ) + + msg, err := stun.Build(setters...) + require.NoError(t, err) + + return msg + } + setupAgent := func(t *testing.T) (*Agent, *pingNoIOCand, *pingNoIOCand, *CandidatePair) { t.Helper() liteAgent := bareAgentForPing() @@ -1643,6 +1676,47 @@ func TestLiteControlledSelector_NoPingCandidate(t *testing.T) { // Still no triggered check emitted assert.Equal(t, uint64(0), pair.RequestsSent()) }) + + t.Run("MalformedNominationOnlyDoesNotNominate", func(t *testing.T) { + agent, local, remote, pair := setupAgent(t) + agent.nominationAttribute = stun.AttrType(0x0030) + require.Equal(t, CandidatePairStateWaiting, pair.state, "pair must start in Waiting") + + ls, ok := agent.getSelector().(*liteSelector) + require.True(t, ok) + + msg := buildMalformedNominationMsg(t, agent, false) + require.False(t, msg.Contains(stun.AttrUseCandidate), "test must not include USE-CANDIDATE") + + ls.HandleBindingRequest(msg, local, remote) + + require.Nil(t, agent.getSelectedPair()) + assert.Equal(t, CandidatePairStateWaiting, pair.state) + assert.False(t, pair.nominated) + assert.False(t, pair.nominateOnBindingSuccess) + assert.Equal(t, uint16(0), pair.bindingRequestCount) + assert.Equal(t, uint64(0), pair.RequestsSent()) + }) + + t.Run("UseCandidateWithMalformedNominationStillAccepted", func(t *testing.T) { + agent, local, remote, pair := setupAgent(t) + agent.nominationAttribute = stun.AttrType(0x0030) + require.Equal(t, CandidatePairStateWaiting, pair.state, "pair must start in Waiting") + + ls, ok := agent.getSelector().(*liteSelector) + require.True(t, ok) + + msg := buildMalformedNominationMsg(t, agent, true) + require.True(t, msg.Contains(stun.AttrUseCandidate), "test must include USE-CANDIDATE") + + ls.HandleBindingRequest(msg, local, remote) + + assert.Equal(t, pair, agent.getSelectedPair()) + assert.Equal(t, CandidatePairStateSucceeded, pair.state) + assert.True(t, pair.nominated) + assert.Equal(t, uint16(0), pair.bindingRequestCount) + assert.Equal(t, uint64(0), pair.RequestsSent()) + }) } // TestLiteMode_FullToLite_Integration is an end-to-end test for the most common