Skip to content

Commit 9117de6

Browse files
committed
message/validation: avoid nil SSVMessage panic in Validate defer
1 parent 34657cf commit 9117de6

File tree

3 files changed

+34
-6
lines changed

3 files changed

+34
-6
lines changed

message/validation/errors.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,6 @@ func (mv *messageValidator) handleValidationError(ctx context.Context, peerID pe
169169
}
170170

171171
func (mv *messageValidator) handleValidationSuccess(ctx context.Context, decodedMessage *queue.SSVMessage) pubsub.ValidationResult {
172-
recordAcceptedMessage(ctx, decodedMessage.GetID().GetRoleType())
172+
recordAcceptedMessage(ctx, messageRole(decodedMessage))
173173
return pubsub.ValidationAccept
174174
}

message/validation/validation.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,7 @@ func (mv *messageValidator) Validate(ctx context.Context, peerID peer.ID, pmsg *
127127
decodedMessage, err := mv.handlePubsubMessage(pmsg, time.Now())
128128

129129
defer func() {
130-
role := spectypes.RunnerRole(spectypes.RoleUnknown)
131-
if decodedMessage != nil {
132-
role = decodedMessage.GetID().GetRoleType()
133-
}
134-
recordMessageDuration(ctx, role, time.Since(validationStart))
130+
recordMessageDuration(ctx, messageRole(decodedMessage), time.Since(validationStart))
135131
}()
136132

137133
if err != nil {
@@ -143,6 +139,13 @@ func (mv *messageValidator) Validate(ctx context.Context, peerID peer.ID, pmsg *
143139
return mv.handleValidationSuccess(ctx, decodedMessage)
144140
}
145141

142+
func messageRole(decodedMessage *queue.SSVMessage) spectypes.RunnerRole {
143+
if decodedMessage == nil || decodedMessage.SSVMessage == nil {
144+
return spectypes.RunnerRole(spectypes.RoleUnknown)
145+
}
146+
return decodedMessage.SSVMessage.GetID().GetRoleType()
147+
}
148+
146149
func (mv *messageValidator) handlePubsubMessage(pMsg *pubsub.Message, receivedAt time.Time) (*queue.SSVMessage, error) {
147150
if err := mv.validatePubSubMessage(pMsg); err != nil {
148151
return nil, err

message/validation/validation_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package validation
22

33
import (
44
"bytes"
5+
"context"
56
"crypto/rsa"
67
"crypto/sha256"
78
"encoding/hex"
@@ -1811,6 +1812,30 @@ func Test_ValidateSSVMessage(t *testing.T) {
18111812
require.ErrorContains(t, err, ErrNilSSVMessage.Error())
18121813
})
18131814

1815+
t.Run("validate handles nil ssv message without panic", func(t *testing.T) {
1816+
validator := New(netCfg, validatorStore, operators, dutyStore, signatureVerifier).(*messageValidator)
1817+
1818+
slot := netCfg.FirstSlotAtEpoch(1)
1819+
signedSSVMessage := generateSignedMessage(ks, committeeIdentifier, slot)
1820+
signedSSVMessage.SSVMessage = nil
1821+
1822+
encoded, err := signedSSVMessage.Encode()
1823+
require.NoError(t, err)
1824+
1825+
topic := "ssv.v2.0"
1826+
pmsg := &pubsub.Message{
1827+
Message: &pspb.Message{
1828+
Data: encoded,
1829+
Topic: &topic,
1830+
},
1831+
}
1832+
1833+
require.NotPanics(t, func() {
1834+
result := validator.Validate(context.Background(), peerID, pmsg)
1835+
require.Equal(t, pubsub.ValidationReject, result)
1836+
})
1837+
})
1838+
18141839
// Receive zero round
18151840
t.Run("zero round", func(t *testing.T) {
18161841
validator := New(netCfg, validatorStore, operators, dutyStore, signatureVerifier).(*messageValidator)

0 commit comments

Comments
 (0)