Ignore malformed nomination attributes

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) {

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