Skip to content

Commit fd0e8fa

Browse files
fix(vote-ext): add unkown fields check (backport GHSA-2fcv-qww3-9v6h) (#1873)
Co-authored-by: Tom <54514587+GAtom22@users.noreply.github.com>
1 parent 02043b1 commit fd0e8fa

4 files changed

Lines changed: 79 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ All notable changes to this project will be documented in this file.
3535

3636
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
3737

38-
## Unreleased
38+
## v4.1.0
39+
40+
### Bug fixes
41+
42+
- [GHSA-2fcv-qww3-9v6h](https://github.com/babylonlabs-io/babylon-ghsa-2fcv-qww3-9v6h/pull/1) Add unkown fields check on vote extension validation
3943

4044
## v4.0.0
4145

test/e2e/govProps/resume-finality.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
"@type": "/babylon.finality.v1.MsgResumeFinalityProposal",
55
"authority": "bbn10d07y265gmmuvt4z0w9aw880jnsr700jduz5f2",
66
"fp_pks_hex": [
7-
"099b2c17480e80b97630f93a80ec4a251e5f9c588eeaa97f964f47c431cda8e7"
7+
"87323407d6f171e21006b75d00ea3c5cc5020799bedf4735286d69f15edd03e1"
88
],
9-
"halting_height": 55
9+
"halting_height": 72
1010
}
1111
],
1212
"metadata": "",

x/checkpointing/prepare/proposal.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
abci "github.com/cometbft/cometbft/abci/types"
1212
"github.com/cosmos/cosmos-sdk/baseapp"
1313
"github.com/cosmos/cosmos-sdk/client"
14+
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
15+
"github.com/cosmos/cosmos-sdk/codec/unknownproto"
1416
sdk "github.com/cosmos/cosmos-sdk/types"
1517
"github.com/cosmos/cosmos-sdk/types/mempool"
1618

@@ -32,8 +34,9 @@ type ProposalHandler struct {
3234
bApp *baseapp.BaseApp
3335

3436
// used for building and parsing the injected tx
35-
txConfig client.TxConfig
36-
mp mempool.Mempool
37+
txConfig client.TxConfig
38+
interfaceRegistry codectypes.InterfaceRegistry
39+
mp mempool.Mempool
3740

3841
defaultPrepareProposalHandler sdk.PrepareProposalHandler
3942
defaultProcessProposalHandler sdk.ProcessProposalHandler
@@ -55,6 +58,7 @@ func NewProposalHandler(
5558
ckptKeeper: ckptKeeper,
5659
bApp: bApp,
5760
txConfig: encCfg.TxConfig,
61+
interfaceRegistry: encCfg.InterfaceRegistry,
5862
defaultPrepareProposalHandler: defaultHandler.PrepareProposalHandler(),
5963
defaultProcessProposalHandler: defaultHandler.ProcessProposalHandler(),
6064
}
@@ -212,6 +216,10 @@ func (h *ProposalHandler) verifyVoteExtension(
212216
}
213217

214218
var ve ckpttypes.VoteExtension
219+
if err := unknownproto.RejectUnknownFieldsStrict(veBytes, &ve, h.interfaceRegistry); err != nil {
220+
return nil, fmt.Errorf("vote extension contains unknown or extra bytes: %w", err)
221+
}
222+
215223
if err := ve.Unmarshal(veBytes); err != nil {
216224
return nil, fmt.Errorf("failed to unmarshal vote extension: %w", err)
217225
}

x/checkpointing/prepare/proposal_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package prepare_test
22

33
import (
44
"bytes"
5+
"encoding/binary"
56
"fmt"
67
"math/rand"
78
"sort"
@@ -696,6 +697,40 @@ func TestPrepareProposalAtVoteExtensionHeight(t *testing.T) {
696697
},
697698
expectError: false,
698699
},
700+
{
701+
name: "Malicious validator with oversized vote extension should be rejected",
702+
scenarioSetup: func(ec *EpochAndCtx, ek *mocks.MockCheckpointingKeeper) *Scenario {
703+
bh := randomBlockHash()
704+
validatorAndExtensions, totalPower := generateNValidatorAndVoteExtensions(t, 4, &bh, ec.Epoch.EpochNumber)
705+
706+
var signedVoteExtensions []cbftt.ExtendedVoteInfo
707+
for i, val := range validatorAndExtensions.Vals {
708+
validator := val
709+
ek.EXPECT().GetPubKeyByConsAddr(gomock.Any(), sdk.ConsAddress(validator.ValidatorAddress(t).Bytes())).Return(validator.ProtoPubkey(), nil).AnyTimes()
710+
ek.EXPECT().VerifyBLSSig(gomock.Any(), validatorAndExtensions.Extensions[i].ToBLSSig()).Return(nil).AnyTimes()
711+
ek.EXPECT().GetBlsPubKey(gomock.Any(), validator.ValidatorAddress(t)).Return(validator.BlsPubKey(), nil).AnyTimes()
712+
marshaledExtension, err := validatorAndExtensions.Extensions[i].Marshal()
713+
require.NoError(t, err)
714+
715+
// First validator adds malicious padding to make the vote extension oversized
716+
if i == 0 {
717+
marshaledExtension = appendProtobufPadding(marshaledExtension, 1*1024*1024-100)
718+
}
719+
720+
signedExtension := validator.SignVoteExtension(t, marshaledExtension, ec.Ctx.HeaderInfo().Height-1, ec.Ctx.ChainID())
721+
signedVoteExtensions = append(signedVoteExtensions, signedExtension)
722+
}
723+
724+
return &Scenario{
725+
TotalPower: totalPower,
726+
ValidatorSet: validatorAndExtensions.Vals,
727+
Extensions: signedVoteExtensions,
728+
TxVerifier: newTxVerifier(encCfg.TxConfig),
729+
ExpectedAbsentVotes: 1, // malicious validator's vote extension should be considered absent
730+
}
731+
},
732+
expectError: false,
733+
},
699734
// TODO: Add scenarios testing compatibility of prepareProposal, processProposal and preBlocker
700735
}
701736

@@ -782,3 +817,30 @@ func TestPrepareProposalAtVoteExtensionHeight(t *testing.T) {
782817
})
783818
}
784819
}
820+
821+
// appendProtobufPadding adds padding bytes to a protobuf message using an unknown field
822+
// This simulates a malicious validator trying to inflate the size of their vote extension
823+
func appendProtobufPadding(data []byte, paddingSize int) []byte {
824+
// Protobuf tag for field 7, wire type 2 (length-delimited)
825+
tag := byte(58) // (7 << 3) | 2
826+
827+
var lengthBuf [binary.MaxVarintLen64]byte
828+
n := binary.PutUvarint(lengthBuf[:], uint64(paddingSize))
829+
830+
result := make([]byte, len(data)+1+n+paddingSize)
831+
832+
copy(result, data)
833+
offset := len(data)
834+
835+
result[offset] = tag
836+
offset++
837+
838+
copy(result[offset:], lengthBuf[:n])
839+
offset += n
840+
841+
for i := 0; i < paddingSize; i++ {
842+
result[offset+i] = byte(i % 256)
843+
}
844+
845+
return result
846+
}

0 commit comments

Comments
 (0)