Skip to content

Commit 223422d

Browse files
authored
fix(network): add vote payload validation in Attest handler (#360)
* security: add vote payload validation in Attest handler The Attest handler previously accepted any vote payload, including empty or garbage data. This allowed attesters to submit meaningless votes that would be counted toward quorum. This commit adds vote payload validation: - Rejects nil/empty vote payloads - Enforces minimum 48-byte length (BLS signature size) - Documents that full cryptographic verification should be added once the vote format is finalized Adds 5 table-driven test cases for vote payload validation. * fix: move minVoteLen to exported constant, remove redundant empty check - MinVoteLen is now a package-level constant (48 bytes) shared between handler and tests to avoid drift - Removed redundant len(msg.Vote) == 0 check since len < 48 covers it - Check SetParams error return (errcheck)
1 parent 2f8d257 commit 223422d

2 files changed

Lines changed: 78 additions & 1 deletion

File tree

modules/network/keeper/msg_server.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ import (
1515
"github.com/evstack/ev-abci/modules/network/types"
1616
)
1717

18+
// MinVoteLen is the minimum vote payload length in bytes.
19+
// 64 is the size of a Ed25519 signature
20+
const MinVoteLen = 64
21+
1822
type msgServer struct {
1923
Keeper
2024
}
@@ -66,7 +70,14 @@ func (k msgServer) Attest(goCtx context.Context, msg *types.MsgAttest) (*types.M
6670
return nil, sdkerr.Wrapf(sdkerrors.ErrInvalidRequest, "consensus address %s already attested for height %d", msg.ConsensusAddress, msg.Height)
6771
}
6872

69-
// TODO: Verify the vote signature here once we implement vote parsing
73+
// Validate vote payload meets minimum signature length.
74+
// A valid vote must contain at least a cryptographic signature (
75+
// 64 bytes for Ed25519). We enforce the minimum here; full cryptographic
76+
// verification of the signature against the block data should be added once
77+
// the vote format is finalized.
78+
if len(msg.Vote) < MinVoteLen {
79+
return nil, sdkerr.Wrapf(sdkerrors.ErrInvalidRequest, "vote payload too short: got %d bytes, minimum %d", len(msg.Vote), MinVoteLen)
80+
}
7081

7182
// Set the bit
7283
k.bitmapHelper.SetBit(bitmap, int(index))

modules/network/keeper/msg_server_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,72 @@ func TestJoinAttesterSetMaxCap(t *testing.T) {
159159
})
160160
}
161161

162+
func TestAttestVotePayloadValidation(t *testing.T) {
163+
myValAddr := sdk.ValAddress("validator1")
164+
165+
specs := map[string]struct {
166+
vote []byte
167+
expErr error
168+
}{
169+
"empty vote rejected": {
170+
vote: []byte{},
171+
expErr: sdkerrors.ErrInvalidRequest,
172+
},
173+
"nil vote rejected": {
174+
vote: nil,
175+
expErr: sdkerrors.ErrInvalidRequest,
176+
},
177+
"short vote rejected": {
178+
vote: make([]byte, MinVoteLen-1),
179+
expErr: sdkerrors.ErrInvalidRequest,
180+
},
181+
"min-length vote accepted": {
182+
vote: make([]byte, MinVoteLen),
183+
},
184+
"valid-length vote accepted": {
185+
vote: make([]byte, 96),
186+
},
187+
}
188+
189+
for name, spec := range specs {
190+
t.Run(name, func(t *testing.T) {
191+
sk := NewMockStakingKeeper()
192+
cdc := moduletestutil.MakeTestEncodingConfig().Codec
193+
keys := storetypes.NewKVStoreKeys(types.StoreKey)
194+
logger := log.NewTestLogger(t)
195+
cms := integration.CreateMultiStore(keys, logger)
196+
authority := authtypes.NewModuleAddress("gov")
197+
keeper := NewKeeper(cdc, runtime.NewKVStoreService(keys[types.StoreKey]), sk, nil, nil, authority.String())
198+
server := msgServer{Keeper: keeper}
199+
ctx := sdk.NewContext(cms, cmtproto.Header{
200+
ChainID: "test-chain",
201+
Time: time.Now().UTC(),
202+
Height: 10,
203+
}, false, logger).WithContext(t.Context())
204+
205+
require.NoError(t, keeper.SetParams(ctx, types.DefaultParams()))
206+
require.NoError(t, keeper.SetAttesterSetMember(ctx, myValAddr.String()))
207+
require.NoError(t, keeper.BuildValidatorIndexMap(ctx))
208+
209+
msg := &types.MsgAttest{
210+
Authority: myValAddr.String(),
211+
ConsensusAddress: myValAddr.String(),
212+
Height: 10,
213+
Vote: spec.vote,
214+
}
215+
216+
rsp, err := server.Attest(ctx, msg)
217+
if spec.expErr != nil {
218+
require.ErrorIs(t, err, spec.expErr)
219+
require.Nil(t, rsp)
220+
return
221+
}
222+
require.NoError(t, err)
223+
require.NotNil(t, rsp)
224+
})
225+
}
226+
}
227+
162228
var _ types.StakingKeeper = &MockStakingKeeper{}
163229

164230
type MockStakingKeeper struct {

0 commit comments

Comments
 (0)