diff --git a/dkg/nodesigs.go b/dkg/nodesigs.go index 7789077da..e1fbe6504 100644 --- a/dkg/nodesigs.go +++ b/dkg/nodesigs.go @@ -126,15 +126,11 @@ func (n *nodeSigBcast) broadcastCallback(ctx context.Context, senderID peer.ID, msgPeerIdx := int(nodeSig.GetPeerIndex()) - sig := nodeSig.GetSignature() - if bytes.Equal(sig, noneData) { - // For certain protocols we allow exchanging nil signatures. - n.setSig(noneData, msgPeerIdx) - - return nil + if msgPeerIdx < 0 || msgPeerIdx >= len(n.peers) { + return errors.New("invalid peer index") } - if (msgPeerIdx == n.nodeIdx.PeerIdx) || (msgPeerIdx < 0 || msgPeerIdx >= len(n.peers)) { + if msgPeerIdx == n.nodeIdx.PeerIdx { return errors.New("invalid peer index") } @@ -143,6 +139,14 @@ func (n *nodeSigBcast) broadcastCallback(ctx context.Context, senderID peer.ID, return errors.New("sender peer ID does not match claimed peer index") } + sig := nodeSig.GetSignature() + if bytes.Equal(sig, noneData) { + // For certain protocols we allow exchanging nil signatures. + n.setSig(noneData, msgPeerIdx) + + return nil + } + lockHash, err := n.lockHash(ctx) if err != nil { return errors.Wrap(err, "lock hash wait") diff --git a/dkg/nodesigs_internal_test.go b/dkg/nodesigs_internal_test.go index 78aec5c69..bd1c6c7d1 100644 --- a/dkg/nodesigs_internal_test.go +++ b/dkg/nodesigs_internal_test.go @@ -245,6 +245,22 @@ func TestSigsCallbacks(t *testing.T) { require.ErrorContains(t, err, "sender peer ID does not match claimed peer index") }) + t.Run("sender ID checked before noneData", func(t *testing.T) { + msg := &dkgpb.MsgNodeSig{ + Signature: noneData, + PeerIndex: uint32(2), // Claims to be from peer 2 + } + + // But actually sent by peer 1 - should fail even with noneData signature + err := ns.broadcastCallback(context.Background(), + peers[1], + "", + msg, + ) + + require.ErrorContains(t, err, "sender peer ID does not match claimed peer index") + }) + t.Run("signature verification failed", func(t *testing.T) { ns.lockHashData = bytes.Repeat([]byte{42}, 32)