diff --git a/sei-tendermint/internal/consensus/memory_limit_test.go b/sei-tendermint/internal/consensus/memory_limit_test.go index 61e193d8dd..935ee4ec6b 100644 --- a/sei-tendermint/internal/consensus/memory_limit_test.go +++ b/sei-tendermint/internal/consensus/memory_limit_test.go @@ -48,6 +48,7 @@ func TestPeerStateMemoryLimits(t *testing.T) { BlockID: blockID, Timestamp: time.Now(), Signature: []byte("test-signature"), + Header: plausibleTestHeader, } ps.SetHasProposal(proposal) if tc.expectError { diff --git a/sei-tendermint/internal/consensus/peer_state_test.go b/sei-tendermint/internal/consensus/peer_state_test.go index 1eeeee6833..7433a72b9b 100644 --- a/sei-tendermint/internal/consensus/peer_state_test.go +++ b/sei-tendermint/internal/consensus/peer_state_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/version" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/libs/log" @@ -12,6 +13,14 @@ import ( "github.com/tendermint/tendermint/types" ) +var plausibleTestHeader = types.Header{ + Version: version.Consensus{ + Block: version.BlockProtocol, + }, + Height: 1, + ProposerAddress: make(types.Address, 20), +} + func peerStateSetup(h, r, v int) *PeerState { ps := NewPeerState(log.NewNopLogger(), "testPeerState") ps.PRS.Height = int64(h) @@ -122,6 +131,7 @@ func TestSetHasProposal(t *testing.T) { Hash: make([]byte, crypto.HashSize), }, }, + Header: plausibleTestHeader, // Missing signature } ps.SetHasProposal(invalidProposal) @@ -142,6 +152,7 @@ func TestSetHasProposal(t *testing.T) { Hash: crypto.CRandBytes(crypto.HashSize), }, }, + Header: plausibleTestHeader, Signature: []byte("signature"), } ps3.SetHasProposal(tooLargeTotalProposal) @@ -160,6 +171,7 @@ func TestSetHasProposal(t *testing.T) { Hash: crypto.CRandBytes(crypto.HashSize), }, }, + Header: plausibleTestHeader, Signature: []byte("signature"), } ps.SetHasProposal(validProposal) @@ -179,6 +191,7 @@ func TestSetHasProposal(t *testing.T) { Hash: crypto.CRandBytes(crypto.HashSize), }, }, + Header: plausibleTestHeader, Signature: []byte("signature"), } ps2.SetHasProposal(differentProposal) @@ -235,6 +248,7 @@ func TestSetHasProposalMemoryLimit(t *testing.T) { // Set up proposal with test case total proposal.BlockID.PartSetHeader.Total = tc.total + proposal.Header = plausibleTestHeader // Use a plausible header // Try to set the proposal ps.SetHasProposal(proposal) @@ -424,6 +438,7 @@ func TestSetHasProposalEdgeCases(t *testing.T) { }, Timestamp: time.Now(), Signature: []byte("test-signature"), + Header: plausibleTestHeader, }, expectProposal: true, // Should be set expectPanic: false, diff --git a/sei-tendermint/internal/consensus/reactor_test.go b/sei-tendermint/internal/consensus/reactor_test.go index 83ab03aea3..4a4faa4f08 100644 --- a/sei-tendermint/internal/consensus/reactor_test.go +++ b/sei-tendermint/internal/consensus/reactor_test.go @@ -842,6 +842,7 @@ func TestReactorMemoryLimitCoverage(t *testing.T) { }, }, Timestamp: time.Now(), + Header: plausibleTestHeader, Signature: []byte("test-signature"), } diff --git a/sei-tendermint/internal/consensus/state_test.go b/sei-tendermint/internal/consensus/state_test.go index 54b4135019..c4b118fe9b 100644 --- a/sei-tendermint/internal/consensus/state_test.go +++ b/sei-tendermint/internal/consensus/state_test.go @@ -2495,8 +2495,22 @@ func TestGossipTransactionKeyOnlyConfig(t *testing.T) { proposalMsg := ProposalMessage{&proposal} peerID, err := types.NewNodeID("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA") startTestRound(ctx, cs1, height, round) - cs1.handleMsg(ctx, msgInfo{&proposalMsg, peerID, time.Now()}, false) + + // Assert invalid proposal is ignored. + invalidProposal := proposal + invalidProposal.Height = -3 + cs1.handleMsg(ctx, msgInfo{&ProposalMessage{&invalidProposal}, peerID, time.Now()}, false) rs := cs1.GetRoundState() + require.Nil(t, rs.Proposal) + require.Nil(t, rs.ProposalBlock) + require.Zero(t, rs.ProposalBlockParts.Total()) + + // Now assert that a valid proposal is processed correctly. + cs1.handleMsg(ctx, msgInfo{&proposalMsg, peerID, time.Now()}, false) + + // GetRoundStates returns a snapshot of the current state. Therefore, we need to + // get it again after processing the proposal message. + rs = cs1.GetRoundState() // Proposal, ProposalBlock and ProposalBlockParts sohuld be set since gossip-tx-key is true if rs.Proposal == nil { t.Error("rs.Proposal should be set") diff --git a/sei-tendermint/types/block.go b/sei-tendermint/types/block.go index 4938de5374..ab32045250 100644 --- a/sei-tendermint/types/block.go +++ b/sei-tendermint/types/block.go @@ -37,6 +37,11 @@ const ( // Uvarint length of Data.Txs: 4 bytes // Data.Txs field: 1 byte MaxOverheadForBlock int64 = 11 + + // MaxCommitSignatures is the maximum number of signatures in a commit. The max + // value is picked relative to the current number of validators and may need to + // be increased in the future as the network grows. + MaxCommitSignatures = 128 ) // Block defines the atomic unit of a Tendermint blockchain. @@ -89,12 +94,7 @@ func (b *Block) ValidateBasic() error { return fmt.Errorf("wrong Header.DataHash. Expected %X, got %X. Len of txs %d", w, g, len(b.Data.Txs)) } - // NOTE: b.Evidence may be nil, but we're just looping. - for i, ev := range b.Evidence { - if err := ev.ValidateBasic(); err != nil { - return fmt.Errorf("invalid evidence (#%d): %v", i, err) - } - } + // Evidence is validated as part of proto decoding. See EvidenceList.FromProto. if w, g := b.Evidence.Hash(), b.EvidenceHash; !bytes.Equal(w, g) { return fmt.Errorf("wrong Header.EvidenceHash. Expected %X, got %X", w, g) @@ -821,6 +821,9 @@ func (commit *Commit) Size() int { // ValidateBasic performs basic validation that doesn't involve state data. // Does not actually check the cryptographic signatures. func (commit *Commit) ValidateBasic() error { + if commit == nil { + return nil + } if commit.Height < 0 { return errors.New("negative Height") } @@ -926,6 +929,10 @@ func CommitFromProto(cp *tmproto.Commit) (*Commit, error) { return nil, err } + if len(cp.Signatures) > MaxCommitSignatures { + return nil, fmt.Errorf("too many signatures in commit: %d (max: %d)", len(cp.Signatures), MaxCommitSignatures) + } + sigs := make([]CommitSig, len(cp.Signatures)) for i := range cp.Signatures { if err := sigs[i].FromProto(cp.Signatures[i]); err != nil { diff --git a/sei-tendermint/types/block_meta.go b/sei-tendermint/types/block_meta.go index b35afd3457..01687767a2 100644 --- a/sei-tendermint/types/block_meta.go +++ b/sei-tendermint/types/block_meta.go @@ -67,9 +67,15 @@ func BlockMetaFromProto(pb *tmproto.BlockMeta) (*BlockMeta, error) { // ValidateBasic performs basic validation. func (bm *BlockMeta) ValidateBasic() error { + if bm == nil { + return errors.New("nil BlockMeta") + } if err := bm.BlockID.ValidateBasic(); err != nil { return err } + if err := bm.Header.ValidateBasic(); err != nil { + return err + } if !bytes.Equal(bm.BlockID.Hash, bm.Header.Hash()) { return fmt.Errorf("expected BlockID#Hash and Header#Hash to be the same, got %X != %X", bm.BlockID.Hash, bm.Header.Hash()) diff --git a/sei-tendermint/types/evidence.go b/sei-tendermint/types/evidence.go index 83cb4b391e..83c41d16a5 100644 --- a/sei-tendermint/types/evidence.go +++ b/sei-tendermint/types/evidence.go @@ -656,7 +656,7 @@ func (evl *EvidenceList) FromProto(eviList *tmproto.EvidenceList) error { eviBzs[i] = evi } *evl = eviBzs - return nil + return evl.ValidateBasic() } // ToProto converts EvidenceList to protobuf @@ -745,6 +745,18 @@ func (evl EvidenceList) ToABCI() []abci.Misbehavior { return el } +func (evl EvidenceList) ValidateBasic() error { + for at, evidence := range evl { + if evidence == nil { + return fmt.Errorf("nil evidence in evidence list at index %d", at) + } + if err := evidence.ValidateBasic(); err != nil { + return fmt.Errorf("invalid evidence at index %d: %w", at, err) + } + } + return nil +} + //------------------------------------------ PROTO -------------------------------------- // EvidenceToProto is a generalized function for encoding evidence that conforms to the diff --git a/sei-tendermint/types/mempool.go b/sei-tendermint/types/mempool.go index a3a0c831eb..b1dedcbec7 100644 --- a/sei-tendermint/types/mempool.go +++ b/sei-tendermint/types/mempool.go @@ -35,6 +35,9 @@ func TxKeyFromProto(dp *tmproto.TxKey) (TxKey, error) { if dp == nil { return TxKey{}, errors.New("nil data") } + if len(dp.TxKey) != sha256.Size { + return TxKey{}, fmt.Errorf("invalid tx key length: %d, expected: %d", len(dp.TxKey), sha256.Size) + } var txBzs [sha256.Size]byte for i := range dp.TxKey { txBzs[i] = dp.TxKey[i] @@ -44,6 +47,9 @@ func TxKeyFromProto(dp *tmproto.TxKey) (TxKey, error) { } func TxKeysListFromProto(dps []*tmproto.TxKey) ([]TxKey, error) { + if len(dps) > maxTxKeysPerProposal { + return nil, fmt.Errorf("too many tx keys in proposal: %d, max: %d", len(dps), maxTxKeysPerProposal) + } var txKeys []TxKey for _, txKey := range dps { txKey, err := TxKeyFromProto(txKey) diff --git a/sei-tendermint/types/node_info.go b/sei-tendermint/types/node_info.go index 3c7758b672..af64bccb8f 100644 --- a/sei-tendermint/types/node_info.go +++ b/sei-tendermint/types/node_info.go @@ -229,7 +229,7 @@ func NodeInfoFromProto(pb *tmp2p.NodeInfo) (NodeInfo, error) { }, } - return dni, nil + return dni, dni.Validate() } // ParseAddressString reads an address string, and returns the IP diff --git a/sei-tendermint/types/proposal.go b/sei-tendermint/types/proposal.go index b8d4d27a05..bbed73f8d6 100644 --- a/sei-tendermint/types/proposal.go +++ b/sei-tendermint/types/proposal.go @@ -3,15 +3,50 @@ package types import ( "errors" "fmt" + "math" "math/bits" + "os" + "strconv" "time" + "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/internal/libs/protoio" tmbytes "github.com/tendermint/tendermint/libs/bytes" tmtime "github.com/tendermint/tendermint/libs/time" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) +// maxTxKeysPerProposal is the maximum number of transaction keys that can be +// included in a proposal. The limit is determined such that the proposal should +// hit the gas limit before ever reaching the max transaction keys in order to +// cap the maximum. +// +// By default, we set this to 1,000 which should be reasonable for most use +// cases. However, this can be overridden by setting the +// SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL environment variable to a positive +// integer value for load testing purposes. +var maxTxKeysPerProposal int + +func init() { + const defaultMaxTxKeysPerProposal = 1_000 + if value, found := os.LookupEnv("SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL"); found { + maxTxKeys, err := strconv.ParseInt(value, 10, 64) + if err != nil { + panic(fmt.Sprintf("Failed to parse SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL %s: %v", value, err)) + } + if maxTxKeys <= 0 { + panic(fmt.Sprintf("SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL must be a positive integer, got %d", maxTxKeys)) + } + if maxTxKeys > math.MaxInt { + panic(fmt.Sprintf("SEI_TENDERMINT_MAX_TX_KEYS_PER_PROPOSAL must be less than or equal to %d, got %d", math.MaxInt, maxTxKeys)) + } + maxTxKeysPerProposal = int(maxTxKeys) + fmt.Printf("Using custom maxTxKeysPerProposal: %d\n", maxTxKeysPerProposal) + } else { + maxTxKeysPerProposal = defaultMaxTxKeysPerProposal + } +} + var ( ErrInvalidBlockPartSignature = errors.New("error invalid block part signature") ErrInvalidBlockPartHash = errors.New("error invalid block part hash") @@ -87,6 +122,28 @@ func (p *Proposal) ValidateBasic() error { if len(p.Signature) > MaxSignatureSize { return fmt.Errorf("signature is too big (max: %d)", MaxSignatureSize) } + + if err := p.LastCommit.ValidateBasic(); err != nil { + return fmt.Errorf("invalid LastCommit: %w", err) + } + + // Evidence is validated as part of proto decoding. See EvidenceList.FromProto. + + if err := p.Header.ValidateBasic(); err != nil { + return fmt.Errorf("invalid Header: %w", err) + } + + if len(p.TxKeys) > maxTxKeysPerProposal { + return fmt.Errorf("invalid number of TxKeys: must be at most %d, got %d", maxTxKeysPerProposal, len(p.TxKeys)) + } + + if len(p.ProposerAddress) != crypto.AddressSize { + return fmt.Errorf( + "invalid ProposerAddress length; got: %d, expected: %d", + len(p.ProposerAddress), crypto.AddressSize, + ) + } + return nil } @@ -228,9 +285,14 @@ func ProposalFromProto(pp *tmproto.Proposal) (*Proposal, error) { } p.Header = header lastCommit, err := CommitFromProto(pp.LastCommit) + if err != nil { + return nil, fmt.Errorf("failed to instantiate last commit: %w", err) + } p.LastCommit = lastCommit eviD := new(EvidenceList) - eviD.FromProto(pp.Evidence) + if err := eviD.FromProto(pp.Evidence); err != nil { + return nil, fmt.Errorf("faield to instantiate evidence list: %w", err) + } p.Evidence = *eviD p.ProposerAddress = pp.ProposerAddress diff --git a/sei-tendermint/types/proposal_test.go b/sei-tendermint/types/proposal_test.go index 16f1ec9b8a..24ee3e24de 100644 --- a/sei-tendermint/types/proposal_test.go +++ b/sei-tendermint/types/proposal_test.go @@ -18,6 +18,8 @@ import ( tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) +var plausibleTestAddress = crypto.Address(make([]byte, crypto.AddressSize)) + func generateHeader() Header { return Header{ Version: version.Consensus{Block: version.BlockProtocol}, @@ -182,6 +184,10 @@ func TestProposalValidateBasic(t *testing.T) { {"Too big Signature", func(p *Proposal) { p.Signature = make([]byte, MaxSignatureSize+1) }, true}, + {"Invalid LastCommit", func(p *Proposal) { p.LastCommit = &Commit{Height: -1} }, true}, + {"Invalid EvidenceList", func(p *Proposal) { p.Evidence = []Evidence{nil} }, true}, + {"Invalid Header", func(p *Proposal) { p.Header = Header{ChainID: string(make([]byte, 100))} }, true}, + {"Too many TxKeys", func(p *Proposal) { p.TxKeys = make([]TxKey, maxTxKeysPerProposal+1) }, true}, } blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), math.MaxInt32, crypto.Checksum([]byte("partshash"))) @@ -208,9 +214,9 @@ func TestProposalValidateBasic(t *testing.T) { func TestProposalProtoBuf(t *testing.T) { var txKeys []TxKey - proposal := NewProposal(1, 2, 3, makeBlockID([]byte("hash"), 2, []byte("part_set_hash")), tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, crypto.Address("testaddr")) + proposal := NewProposal(1, 2, 3, makeBlockID([]byte("hash"), 2, []byte("part_set_hash")), tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, plausibleTestAddress) proposal.Signature = []byte("sig") - proposal2 := NewProposal(1, 2, 3, BlockID{}, tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, crypto.Address("testaddr")) + proposal2 := NewProposal(1, 2, 3, BlockID{}, tmtime.Now(), txKeys, generateHeader(), &Commit{Signatures: []CommitSig{}}, EvidenceList{}, plausibleTestAddress) testCases := []struct { msg string diff --git a/sei-tendermint/types/validator.go b/sei-tendermint/types/validator.go index b542037f20..5ffe1b0891 100644 --- a/sei-tendermint/types/validator.go +++ b/sei-tendermint/types/validator.go @@ -209,7 +209,7 @@ func ValidatorFromProto(vp *tmproto.Validator) (*Validator, error) { v.VotingPower = vp.GetVotingPower() v.ProposerPriority = vp.GetProposerPriority() - return v, nil + return v, v.ValidateBasic() } //---------------------------------------- diff --git a/sei-tendermint/types/vote.go b/sei-tendermint/types/vote.go index 40f829182d..e759e4c029 100644 --- a/sei-tendermint/types/vote.go +++ b/sei-tendermint/types/vote.go @@ -68,7 +68,7 @@ func VoteFromProto(pv *tmproto.Vote) (*Vote, error) { return nil, err } - return &Vote{ + v := &Vote{ Type: pv.Type, Height: pv.Height, Round: pv.Round, @@ -77,7 +77,8 @@ func VoteFromProto(pv *tmproto.Vote) (*Vote, error) { ValidatorAddress: pv.ValidatorAddress, ValidatorIndex: pv.ValidatorIndex, Signature: pv.Signature, - }, nil + } + return v, v.ValidateBasic() } // CommitSig converts the Vote to a CommitSig. diff --git a/sei-tendermint/types/vote_test.go b/sei-tendermint/types/vote_test.go index 3ac651ce35..4d46b8b598 100644 --- a/sei-tendermint/types/vote_test.go +++ b/sei-tendermint/types/vote_test.go @@ -301,7 +301,7 @@ func TestVoteProtobuf(t *testing.T) { passesValidateBasic bool }{ {"success", vote, true, true}, - {"fail vote validate basic", &Vote{}, true, false}, + {"fail vote validate basic", &Vote{}, false, false}, } for _, tc := range testCases { protoProposal := tc.vote.ToProto()