From e432713e8c6374ab63d47d67aa0b47271ce2ee75 Mon Sep 17 00:00:00 2001 From: Solovyov1796 Date: Fri, 19 Jun 2026 01:45:22 +0800 Subject: [PATCH 1/2] fix(consensus/reactor): reject oversized proposals (backport #5324) (#5407) --- Updates the consensus reactor to validate that a received proposal will not contain more parts than the amount of chunks that it would take to build a block whos size is equal to `ConsensusParams.Block.MaxBytes`. Original PR is here #5309, but reopened since the contributor stopped replying. #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments
This is an automatic backport of pull request #5324 done by [Mergify](https://mergify.com). Co-authored-by: Alex | Interchain Labs Co-authored-by: arsushi Co-authored-by: Abdul Malek Co-authored-by: Matt Acciai Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com> --- internal/consensus/byzantine_test.go | 83 ++++++++++++++++++++++++++++ internal/consensus/reactor.go | 15 +++++ types/proposal.go | 16 ++++++ types/proposal_test.go | 28 ++++++++++ 4 files changed, 142 insertions(+) diff --git a/internal/consensus/byzantine_test.go b/internal/consensus/byzantine_test.go index 146a49622..396220088 100644 --- a/internal/consensus/byzantine_test.go +++ b/internal/consensus/byzantine_test.go @@ -594,3 +594,86 @@ func (br *ByzantineReactor) Receive(e p2p.Envelope) { } func (*ByzantineReactor) InitPeer(peer p2p.Peer) p2p.Peer { return peer } + +// Large/oversized proposals should be rejected +func TestRejectOversizedProposals(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + n := 2 + css, cleanup := randConsensusNet(t, n, "consensus_reactor_test", newMockTickerFunc(false), newKVStore) + defer cleanup() + + switches := make([]*p2p.Switch, n) + p2pLogger := consensusLogger().With("module", "p2p") + for i := 0; i < n; i++ { + switches[i] = p2p.MakeSwitch( + config.P2P, + i, + func(_ int, sw *p2p.Switch) *p2p.Switch { + return sw + }) + switches[i].SetLogger(p2pLogger.With("validator", i)) + } + + reactors := make([]p2p.Reactor, n) + for i := 0; i < n; i++ { + conR := NewReactor(css[i], false) + defer func() { require.NoError(t, conR.Stop()) }() + + conR.SetLogger(consensusLogger().With("validator", i)) + reactors[i] = conR + } + + p2p.MakeConnectedSwitches(config.P2P, n, func(i int, _ *p2p.Switch) *p2p.Switch { + switches[i].AddReactor("CONSENSUS", reactors[i]) + return switches[i] + }, p2p.Connect2Switches) + + peers := switches[0].Peers().Copy() + targetPeer := peers[0] + + height := int64(1) + round := int32(0) + cs := css[0] + + block, err := cs.createProposalBlock(ctx) + require.NoError(t, err) + + blockParts, err := block.MakePartSet(types.BlockPartSizeBytes) + require.NoError(t, err) + + // create oversized proposal + propBlockID := types.BlockID{Hash: block.Hash(), PartSetHeader: blockParts.Header()} + propBlockID.PartSetHeader.Total = 4294967295 + + proposal := types.NewProposal(height, round, -1, propBlockID, block.Header.Time) + p := proposal.ToProto() + if err := cs.privValidator.SignProposal(cs.state.ChainID, p); err != nil { + t.Error(err) + } + proposal.Signature = p.Signature + + success := targetPeer.Send(p2p.Envelope{ + ChannelID: DataChannel, + Message: &cmtcons.Proposal{Proposal: *proposal.ToProto()}, + }) + require.True(t, success) + + select { + case e := <-css[1].peerMsgQueue: + // if we receive a message here, the peer incorrectly accepted the + // oversized proposal + if _, receivedProposal := e.Msg.(*ProposalMessage); receivedProposal { + assert.Fail(t, "peer incorrectly accepted oversized proposal") + return + } + // invalid state, we received some other unexpected message type, fail + // the test + assert.Fail(t, "received unexpected message type on peer msg queue, expected *ProposalMessage") + case <-ctx.Done(): + case <-time.After(500 * time.Millisecond): + // timeout after 500ms if nothing has happened and assume peer rejected + // the proposal + } +} diff --git a/internal/consensus/reactor.go b/internal/consensus/reactor.go index 909bfd8df..60adc8a3b 100644 --- a/internal/consensus/reactor.go +++ b/internal/consensus/reactor.go @@ -333,6 +333,15 @@ func (conR *Reactor) Receive(e p2p.Envelope) { } switch msg := msg.(type) { case *ProposalMessage: + conR.conS.mtx.RLock() + maxBytes := conR.conS.state.ConsensusParams.Block.MaxBytes + conR.conS.mtx.RUnlock() + if err := msg.Proposal.ValidateBlockSize(maxBytes); err != nil { + conR.Logger.Error("Rejecting oversized proposal", "peer", e.Src, "height", msg.Proposal.Height) + conR.Switch.StopPeerForError(e.Src, ErrProposalTooManyParts) + return + } + ps.SetHasProposal(msg.Proposal) conR.conS.peerMsgQueue <- msgInfo{msg, e.Src.ID(), cmttime.Now()} case *ProposalPOLMessage: @@ -1861,6 +1870,12 @@ func (m *ProposalMessage) ValidateBasic() error { return m.Proposal.ValidateBasic() } +// ValidateBlockSize validates the proposals block size against a maximum. If +// -1 is passed, types.MaxBlockSizeBytes will be used as the maximum. +func (m *ProposalMessage) ValidateBlockSize(maxBlockSizeBytes int64) error { + return m.Proposal.ValidateBlockSize(maxBlockSizeBytes) +} + // String returns a string representation. func (m *ProposalMessage) String() string { return fmt.Sprintf("[Proposal %v]", m.Proposal) diff --git a/types/proposal.go b/types/proposal.go index bce67565d..438fca398 100644 --- a/types/proposal.go +++ b/types/proposal.go @@ -82,6 +82,22 @@ func (p *Proposal) ValidateBasic() error { return nil } +// ValidateBlockSize block size ensures that a proposal block is not larger +// than a maximum number of bytes, based on the total amount of parts reported +// in the PartSetHeader. If -1 is passed as the maxBlockSizeBytes, +// types.MaxBlockSizeBytes will be used as the maximum. +func (p *Proposal) ValidateBlockSize(maxBlockSizeBytes int64) error { + if maxBlockSizeBytes == -1 { + maxBlockSizeBytes = int64(MaxBlockSizeBytes) + } + totalParts := int64(p.BlockID.PartSetHeader.Total) + maxParts := (maxBlockSizeBytes-1)/int64(BlockPartSizeBytes) + 1 + if totalParts > maxParts { + return fmt.Errorf("proposal has too many parts %d (max: %d)", totalParts, maxParts) + } + return nil +} + // IsTimely validates that the proposal timestamp is 'timely' according to the // proposer-based timestamp algorithm. To evaluate if a proposal is timely, its // timestamp is compared to the local time of the validator when it receives diff --git a/types/proposal_test.go b/types/proposal_test.go index 8c82a5547..a9d073dc6 100644 --- a/types/proposal_test.go +++ b/types/proposal_test.go @@ -346,3 +346,31 @@ func TestProposalIsTimelyOverflow(t *testing.T) { p.Timestamp = timestamp.Add(-sp.MessageDelay).Add(-sp.Precision) assert.True(t, p.IsTimely(proposalReceiveTime, sp)) } + +func TestProposalValidateBlockSize(t *testing.T) { + now := time.Now() + testCases := []struct { + testName string + maxBlockSize int64 + proposal *Proposal + expectPass bool + }{ + {"10 chunk max, 5 chunk proposal, success", int64(10 * BlockPartSizeBytes), NewProposal(0, 0, 0, BlockID{PartSetHeader: PartSetHeader{Total: 5}}, now), true}, + {"10 chunk max, 20 chunk proposal, fail", int64(10 * BlockPartSizeBytes), NewProposal(0, 0, 0, BlockID{PartSetHeader: PartSetHeader{Total: 20}}, now), false}, + {"10 chunk max, max uint32 chunk proposal, fail", int64(10 * BlockPartSizeBytes), NewProposal(0, 0, 0, BlockID{PartSetHeader: PartSetHeader{Total: math.MaxUint32}}, now), false}, + {"-1 chunk max, max uint32 chunk proposal, fail", -1, NewProposal(0, 0, 0, BlockID{PartSetHeader: PartSetHeader{Total: math.MaxUint32}}, now), false}, + {"0 chunk max, max uint32 chunk proposal, fail", -1, NewProposal(0, 0, 0, BlockID{PartSetHeader: PartSetHeader{Total: math.MaxUint32}}, now), false}, + {"total parts equals chunk max, success", -1, NewProposal(0, 0, 0, BlockID{PartSetHeader: PartSetHeader{Total: 1600}}, now), true}, + } + for _, tc := range testCases { + tc := tc + t.Run(tc.testName, func(t *testing.T) { + err := tc.proposal.ValidateBlockSize(tc.maxBlockSize) + if tc.expectPass { + require.NoError(t, err) + } else { + require.Error(t, err) + } + }) + } +} From f3819be1aa1bfc18a6b4517f5fbbe95b5f678d6d Mon Sep 17 00:00:00 2001 From: Solovyov1796 Date: Thu, 25 Jun 2026 15:55:48 +0800 Subject: [PATCH 2/2] Assign the local time to the `timestamp` field in `Vote`. --- internal/consensus/state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/consensus/state.go b/internal/consensus/state.go index 9ada0f66e..c2eae3037 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -2672,7 +2672,7 @@ func (cs *State) signVote( Round: cs.Round, Type: msgType, BlockID: types.BlockID{Hash: hash, PartSetHeader: header}, - Timestamp: time.Time{}, + Timestamp: time.Now(), } extEnabled := cs.isVoteExtensionsEnabled(vote.Height)