Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions blocksync/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

bcproto "github.com/tendermint/tendermint/proto/tendermint/blocksync"
"github.com/tendermint/tendermint/types"
"github.com/tendermint/tendermint/upgrade"
)

const (
Expand Down Expand Up @@ -88,23 +87,13 @@ func ValidateMsg(pb proto.Message) error {
return errors.New("negative Height")
}
case *bcproto.BlockResponse:
// V1 block format
block, err := types.BlockFromProto(msg.Block)
if err != nil {
if _, err := types.BlockFromProto(msg.Block); err != nil {
return err
}
if upgrade.IsUpgraded(block.Height) {
return fmt.Errorf("unexpected BlockResponse at upgraded height %d", block.Height)
}
case *bcproto.BlockResponseV2:
// V2 block format (sequencer mode)
block, err := types.BlockV2FromProto(msg.Block)
if err != nil {
if _, err := types.BlockV2FromProto(msg.Block); err != nil {
return err
}
if !upgrade.IsUpgraded(block.GetHeight()) {
return fmt.Errorf("unexpected BlockResponseV2 before upgraded height %d", block.GetHeight())
}
case *bcproto.NoBlockResponse:
if msg.Height < 0 {
return errors.New("negative Height")
Expand Down
68 changes: 12 additions & 56 deletions blocksync/msgs_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package blocksync_test

import (
"bytes"
"encoding/hex"
"math"
"math/big"
Expand All @@ -13,10 +12,8 @@ import (
"github.com/stretchr/testify/require"

"github.com/tendermint/tendermint/blocksync"
"github.com/tendermint/tendermint/crypto"
bcproto "github.com/tendermint/tendermint/proto/tendermint/blocksync"
"github.com/tendermint/tendermint/types"
"github.com/tendermint/tendermint/upgrade"
)

func TestBcBlockRequestMessageValidateBasic(t *testing.T) {
Expand Down Expand Up @@ -84,65 +81,24 @@ func TestBcStatusResponseMessageValidateBasic(t *testing.T) {
}
}

func TestValidateMsgBlockResponseRejectsV1AtUpgradeHeight(t *testing.T) {
oldHeight := upgrade.UpgradeBlockHeight
upgrade.SetUpgradeBlockHeight(10)
defer upgrade.SetUpgradeBlockHeight(oldHeight)

lastBlockID := types.BlockID{
Hash: bytes.Repeat([]byte{1}, 32),
PartSetHeader: types.PartSetHeader{
Total: 1,
Hash: bytes.Repeat([]byte{2}, 32),
},
}
lastCommit := types.NewCommit(9, 0, lastBlockID, []types.CommitSig{{
BlockIDFlag: types.BlockIDFlagCommit,
ValidatorAddress: bytes.Repeat([]byte{3}, crypto.AddressSize),
Signature: []byte{1},
}})

block := types.MakeBlock(10, []types.Tx{types.Tx("Hello World")}, nil, nil, nil, lastCommit, nil)
block.ProposerAddress = bytes.Repeat([]byte{4}, crypto.AddressSize)
bpb, err := block.ToProto()
require.NoError(t, err)

err = blocksync.ValidateMsg(&bcproto.BlockResponse{Block: bpb})
require.Error(t, err)
assert.Contains(t, err.Error(), "unexpected BlockResponse")
}

func TestValidateMsgBlockResponseV2RejectsPreUpgradeHeight(t *testing.T) {
oldHeight := upgrade.UpgradeBlockHeight
upgrade.SetUpgradeBlockHeight(10)
defer upgrade.SetUpgradeBlockHeight(oldHeight)

blockV2 := &types.BlockV2{
ParentHash: common.HexToHash("0x1"),
Hash: common.HexToHash("0x2"),
BaseFee: big.NewInt(1),
Number: 9,
}

err := blocksync.ValidateMsg(&bcproto.BlockResponseV2{Block: types.BlockV2ToProto(blockV2)})
require.Error(t, err)
assert.Contains(t, err.Error(), "unexpected BlockResponseV2")
}

func TestValidateMsgBlockResponseV2AllowsUpgradeHeight(t *testing.T) {
oldHeight := upgrade.UpgradeBlockHeight
upgrade.SetUpgradeBlockHeight(10)
defer upgrade.SetUpgradeBlockHeight(oldHeight)

// ValidateMsg validates block responses structurally only. The old "reject V1/V2 by upgrade
// height" checks were removed: the timestamp-driven boundary is unknown at message-validation
// time, so the blocksync reactor guards the V1/V2 type at consume time instead. A structurally
// valid block passes regardless of height; a malformed one is rejected.
func TestValidateMsgBlockResponseV2Structural(t *testing.T) {
blockV2 := &types.BlockV2{
ParentHash: common.HexToHash("0x1"),
Hash: common.HexToHash("0x2"),
Signature: make([]byte, 65),
BaseFee: big.NewInt(1),
Number: 10,
}
require.NoError(t, blocksync.ValidateMsg(&bcproto.BlockResponseV2{Block: types.BlockV2ToProto(blockV2)}))

err := blocksync.ValidateMsg(&bcproto.BlockResponseV2{Block: types.BlockV2ToProto(blockV2)})
require.NoError(t, err)
// Malformed V2 (bad hash length) -> rejected by structural validation.
bad := types.BlockV2ToProto(blockV2)
bad.Hash = []byte{0x1, 0x2}
require.Error(t, blocksync.ValidateMsg(&bcproto.BlockResponseV2{Block: bad}))
}

//nolint:lll // ignore line length in tests
Expand All @@ -164,7 +120,7 @@ func TestBlockchainMessageVectors(t *testing.T) {
BlockRequest: &bcproto.BlockRequest{Height: math.MaxInt64}}},
"0a0a08ffffffffffffffff7f"},
{"BlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_BlockResponse{
BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1a700a6e0a5b0a02080b1803220b088092b8c398feffffff012a0212003a20c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf96a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855120d0a0b48656c6c6f20576f726c641a00"},
BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1a700a6e0a5b0a02080b1803220b088092b8c398feffffff012a021a003a20fb5fddfb1922fa75ac66398d8adeb962739e0259e7b3e811a81449683a3b78cd6a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855120d0a0b48656c6c6f20576f726c641a00"},
{"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{
NoBlockResponse: &bcproto.NoBlockResponse{Height: 1}}}, "12020801"},
{"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{
Expand Down
22 changes: 15 additions & 7 deletions blocksync/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,39 +335,47 @@ func (pool *BlockPool) GetPeerIDs() []p2p.ID {
// It also enforces basic sanity checks against malicious or buggy peers
// (see spec-003-blocksync-malicious-peer-fix):
//
// - A peer reporting base > height is structurally impossible and is banned
// - A peer reporting base > height is structurally impossible and is rejected
// (CometBFT issue #5801 hardening).
// - An existing peer reporting a strictly lower height or base than what it
// reported previously is removed from the pool and banned to prevent
// reported previously is removed from the pool to prevent
// poisoning of maxPeerHeight (CVE-2025-24371).
// - Non-persistent peers that report invalid ranges are also banned.
// Persistent peers are removed but not added to bannedPeers so the switch
// can reconnect them later without accepting the invalid range.
// - A peer that has been banned recently is silently ignored if it tries to
// re-introduce itself before its ban expires.
//
// In all non-malicious cases, maxPeerHeight is recomputed via
// updateMaxPeerHeight, which filters peers whose base is above pool.height.
func (pool *BlockPool) SetPeerRange(peerID p2p.ID, base int64, height int64) {
func (pool *BlockPool) SetPeerRange(srcPeer p2p.Peer, base int64, height int64) {
peerID := srcPeer.ID()
pool.mtx.Lock()
defer pool.mtx.Unlock()

if base > height {
pool.Logger.Info("Peer reporting base greater than height; banning",
pool.Logger.Info("Peer reporting base greater than height; removing",
"peer", peerID, "base", base, "height", height)
if _, exists := pool.peers[peerID]; exists {
pool.removePeer(peerID)
}
pool.banPeer(peerID)
if !srcPeer.IsPersistent() {
pool.banPeer(peerID)
}
return
}

peer := pool.peers[peerID]
if peer != nil {
if height < peer.height || base < peer.base {
pool.Logger.Info("Peer reporting decreasing height or base; removing and banning",
pool.Logger.Info("Peer reporting decreasing height or base; removing",
"peer", peerID,
"prevHeight", peer.height, "height", height,
"prevBase", peer.base, "base", base)
pool.removePeer(peerID)
pool.banPeer(peerID)
if !srcPeer.IsPersistent() {
pool.banPeer(peerID)
}
return
}
peer.base = base
Expand Down
Loading
Loading