Skip to content

Commit

Permalink
imp(staking): detect the length of the ed25519 pubkey in CreateValida…
Browse files Browse the repository at this point in the history
…tor to prevent panic
  • Loading branch information
luchenqun committed Nov 19, 2023
1 parent c3308d3 commit 8a9ebb7
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [#17733](https://github.com/cosmos/cosmos-sdk/pull/17733) Ensure `buf export` exports all proto dependencies
* (version) [#18063](https://github.com/cosmos/cosmos-sdk/pull/18063) Include additional information in the Info struct. This change enhances the Info struct by adding support for additional information through the ExtraInfo field
* (crypto | x/auth) [#14372](https://github.com/cosmos/cosmos-sdk/pull/18194) Key checks on signatures antehandle.
* (staking) [#18506](https://github.com/cosmos/cosmos-sdk/pull/18506) Detect the length of the ed25519 pubkey in CreateValidator to prevent panic.

### Bug Fixes

Expand Down
46 changes: 27 additions & 19 deletions x/staking/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"cosmossdk.io/math"
"cosmossdk.io/x/staking/types"

"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -63,25 +64,6 @@ func (k msgServer) CreateValidator(ctx context.Context, msg *types.MsgCreateVali
return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidType, "Expecting cryptotypes.PubKey, got %T", pk)
}

if _, err := k.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(pk)); err == nil {
return nil, types.ErrValidatorPubKeyExists
}

bondDenom, err := k.BondDenom(ctx)
if err != nil {
return nil, err
}

if msg.Value.Denom != bondDenom {
return nil, errorsmod.Wrapf(
sdkerrors.ErrInvalidRequest, "invalid coin denomination: got %s, expected %s", msg.Value.Denom, bondDenom,
)
}

if _, err := msg.Description.EnsureLength(); err != nil {
return nil, err
}

sdkCtx := sdk.UnwrapSDKContext(ctx)
cp := sdkCtx.ConsensusParams()
if cp.Validator != nil {
Expand All @@ -99,6 +81,32 @@ func (k msgServer) CreateValidator(ctx context.Context, msg *types.MsgCreateVali
"got: %s, expected: %s", pk.Type(), cp.Validator.PubKeyTypes,
)
}

if pkType == "ed25519" && len(pk.Bytes()) != ed25519.PubKeySize {
return nil, errorsmod.Wrapf(
types.ErrConsensusPubKeyLenInvalid,
"got: %d, expected: %d", len(pk.Bytes()), ed25519.PubKeySize,
)
}
}

if _, err := k.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(pk)); err == nil {
return nil, types.ErrValidatorPubKeyExists
}

bondDenom, err := k.BondDenom(ctx)
if err != nil {
return nil, err
}

if msg.Value.Denom != bondDenom {
return nil, errorsmod.Wrapf(
sdkerrors.ErrInvalidRequest, "invalid coin denomination: got %s, expected %s", msg.Value.Denom, bondDenom,
)
}

if _, err := msg.Description.EnsureLength(); err != nil {
return nil, err
}

validator, err := types.NewValidator(msg.ValidatorAddress, pk, msg.Description)
Expand Down
32 changes: 32 additions & 0 deletions x/staking/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"
"time"

cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/golang/mock/gomock"

"cosmossdk.io/collections"
Expand All @@ -14,6 +15,7 @@ import (
"github.com/cosmos/cosmos-sdk/codec/address"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
)
Expand All @@ -40,6 +42,16 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() {
pubkey, err := codectypes.NewAnyWithValue(pk1)
require.NoError(err)

var ed25519pk cryptotypes.PubKey = &ed25519.PubKey{Key: []byte{1, 2, 3, 4, 5, 6}}
pubkeyInvalidLen, err := codectypes.NewAnyWithValue(ed25519pk)
require.NoError(err)

ctx = ctx.WithConsensusParams(cmtproto.ConsensusParams{
Validator: &cmtproto.ValidatorParams{
PubKeyTypes: []string{"ed25519"},
},
})

testCases := []struct {
name string
input *stakingtypes.MsgCreateValidator
Expand Down Expand Up @@ -104,6 +116,26 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() {
expErr: true,
expErrMsg: "empty validator public key",
},
{
name: "validator pubkey len is invalid",
input: &stakingtypes.MsgCreateValidator{
Description: stakingtypes.Description{
Moniker: "NewValidator",
},
Commission: stakingtypes.CommissionRates{
Rate: math.LegacyNewDecWithPrec(5, 1),
MaxRate: math.LegacyNewDecWithPrec(5, 1),
MaxChangeRate: math.LegacyNewDec(0),
},
MinSelfDelegation: math.NewInt(1),
DelegatorAddress: Addr.String(),
ValidatorAddress: ValAddr.String(),
Pubkey: pubkeyInvalidLen,
Value: sdk.NewInt64Coin("stake", 10000),
},
expErr: true,
expErrMsg: "consensus pubkey len is invalid",
},
{
name: "empty delegation amount",
input: &stakingtypes.MsgCreateValidator{
Expand Down
1 change: 1 addition & 0 deletions x/staking/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,5 @@ var (
// consensus key errors
ErrConsensusPubKeyAlreadyUsedForValidator = errors.Register(ModuleName, 46, "consensus pubkey is already used for a validator")
ErrExceedingMaxConsPubKeyRotations = errors.Register(ModuleName, 47, "exceeding maximum consensus pubkey rotations within unbonding period")
ErrConsensusPubKeyLenInvalid = errors.Register(ModuleName, 48, "consensus pubkey len is invalid")
)

0 comments on commit 8a9ebb7

Please sign in to comment.