From ec4af4751ddaff4edb5a835d36552354204a9392 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 12 Mar 2026 20:56:07 +0100 Subject: [PATCH 1/3] fix(ante): check nested authz --- ante/gov_vote_ante.go | 199 ++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 97 deletions(-) diff --git a/ante/gov_vote_ante.go b/ante/gov_vote_ante.go index e38c4bd9..11c5fe15 100644 --- a/ante/gov_vote_ante.go +++ b/ante/gov_vote_ante.go @@ -1,6 +1,7 @@ package ante import ( + "context" "errors" errorsmod "cosmossdk.io/errors" @@ -61,123 +62,127 @@ func (g GovVoteDecorator) AnteHandle( // ValidateVoteMsgs checks if a voter has enough stake to vote func (g GovVoteDecorator) ValidateVoteMsgs(ctx sdk.Context, msgs []sdk.Msg) error { - validMsg := func(m sdk.Msg) error { - var accAddr sdk.AccAddress - var err error - - switch msg := m.(type) { - case *govv1beta1.MsgVote: - accAddr, err = sdk.AccAddressFromBech32(msg.Voter) - if err != nil { - return err - } - case *govv1.MsgVote: - accAddr, err = sdk.AccAddressFromBech32(msg.Voter) - if err != nil { - return err - } - case *govv1beta1.MsgVoteWeighted: - accAddr, err = sdk.AccAddressFromBech32(msg.Voter) - if err != nil { - return err - } - case *govv1.MsgVoteWeighted: - accAddr, err = sdk.AccAddressFromBech32(msg.Voter) - if err != nil { - return err - } - case *sdkgovv1beta1.MsgVote: - accAddr, err = sdk.AccAddressFromBech32(msg.Voter) - if err != nil { - return err - } - case *sdkgovv1.MsgVote: - accAddr, err = sdk.AccAddressFromBech32(msg.Voter) - if err != nil { - return err - } - case *sdkgovv1beta1.MsgVoteWeighted: - accAddr, err = sdk.AccAddressFromBech32(msg.Voter) - if err != nil { - return err - } - case *sdkgovv1.MsgVoteWeighted: - accAddr, err = sdk.AccAddressFromBech32(msg.Voter) - if err != nil { + for _, m := range msgs { + if msg, ok := m.(*authz.MsgExec); ok { + if err := g.validAuthz(ctx, msg); err != nil { return err } - default: - // not a vote message - nothing to validate - return nil + continue } - if minStakedTokens.IsZero() { - return nil + // validate normal msgs + if err := g.validMsg(ctx, m); err != nil { + return err } + } + return nil +} - enoughStake := false - delegationCount := 0 - stakedTokens := math.LegacyNewDec(0) - if err := g.stakingKeeper.IterateDelegatorDelegations(ctx, accAddr, func(delegation stakingtypes.Delegation) bool { - validatorAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress) - if err != nil { - panic(err) // shouldn't happen - } - validator, err := g.stakingKeeper.GetValidator(ctx, validatorAddr) - if errors.Is(err, stakingtypes.ErrNoValidatorFound) { - return false - } else if err != nil { - panic(err) // shouldn't happen - } +func (g GovVoteDecorator) validAuthz(ctx context.Context, execMsg *authz.MsgExec) error { + for _, v := range execMsg.Msgs { + var innerMsg sdk.Msg + if err := g.cdc.UnpackAny(v, &innerMsg); err != nil { + return errorsmod.Wrap(atomoneerrors.ErrUnauthorized, "cannot unmarshal authz exec msgs") + } + // Reject nested MsgExec to prevent bypassing stake checks + if _, ok := innerMsg.(*authz.MsgExec); ok { + return g.validAuthz(ctx, execMsg) + } + if err := g.validMsg(ctx, innerMsg); err != nil { + return err + } + } - shares := delegation.Shares - tokens := validator.TokensFromSharesTruncated(shares) - stakedTokens = stakedTokens.Add(tokens) - if stakedTokens.GTE(minStakedTokens) { - enoughStake = true - return true // break the iteration - } + return nil +} + +func (g GovVoteDecorator) validMsg(ctx context.Context, m sdk.Msg) error { + var accAddr sdk.AccAddress + var err error - delegationCount++ - // break the iteration if maxDelegationsChecked were already checked - return delegationCount >= maxDelegationsChecked - }); err != nil { + switch msg := m.(type) { + case *govv1beta1.MsgVote: + accAddr, err = sdk.AccAddressFromBech32(msg.Voter) + if err != nil { return err } - - if !enoughStake { - return errorsmod.Wrapf(atomoneerrors.ErrInsufficientStake, "insufficient stake for voting - min required %v", minStakedTokens) + case *govv1.MsgVote: + accAddr, err = sdk.AccAddressFromBech32(msg.Voter) + if err != nil { + return err } - + case *govv1beta1.MsgVoteWeighted: + accAddr, err = sdk.AccAddressFromBech32(msg.Voter) + if err != nil { + return err + } + case *govv1.MsgVoteWeighted: + accAddr, err = sdk.AccAddressFromBech32(msg.Voter) + if err != nil { + return err + } + case *sdkgovv1beta1.MsgVote: + accAddr, err = sdk.AccAddressFromBech32(msg.Voter) + if err != nil { + return err + } + case *sdkgovv1.MsgVote: + accAddr, err = sdk.AccAddressFromBech32(msg.Voter) + if err != nil { + return err + } + case *sdkgovv1beta1.MsgVoteWeighted: + accAddr, err = sdk.AccAddressFromBech32(msg.Voter) + if err != nil { + return err + } + case *sdkgovv1.MsgVoteWeighted: + accAddr, err = sdk.AccAddressFromBech32(msg.Voter) + if err != nil { + return err + } + default: + // not a vote message - nothing to validate return nil } - validAuthz := func(execMsg *authz.MsgExec) error { - for _, v := range execMsg.Msgs { - var innerMsg sdk.Msg - if err := g.cdc.UnpackAny(v, &innerMsg); err != nil { - return errorsmod.Wrap(atomoneerrors.ErrUnauthorized, "cannot unmarshal authz exec msgs") - } - if err := validMsg(innerMsg); err != nil { - return err - } - } - + if minStakedTokens.IsZero() { return nil } - for _, m := range msgs { - if msg, ok := m.(*authz.MsgExec); ok { - if err := validAuthz(msg); err != nil { - return err - } - continue + enoughStake := false + delegationCount := 0 + stakedTokens := math.LegacyNewDec(0) + if err := g.stakingKeeper.IterateDelegatorDelegations(ctx, accAddr, func(delegation stakingtypes.Delegation) bool { + validatorAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress) + if err != nil { + panic(err) // shouldn't happen + } + validator, err := g.stakingKeeper.GetValidator(ctx, validatorAddr) + if errors.Is(err, stakingtypes.ErrNoValidatorFound) { + return false + } else if err != nil { + panic(err) // shouldn't happen } - // validate normal msgs - if err := validMsg(m); err != nil { - return err + shares := delegation.Shares + tokens := validator.TokensFromSharesTruncated(shares) + stakedTokens = stakedTokens.Add(tokens) + if stakedTokens.GTE(minStakedTokens) { + enoughStake = true + return true // break the iteration } + + delegationCount++ + // break the iteration if maxDelegationsChecked were already checked + return delegationCount >= maxDelegationsChecked + }); err != nil { + return err + } + + if !enoughStake { + return errorsmod.Wrapf(atomoneerrors.ErrInsufficientStake, "insufficient stake for voting - min required %v", minStakedTokens) } + return nil } From ad5fb858afdad9ff4780a190347a1b70d8b42407 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Mar 2026 16:04:08 +0100 Subject: [PATCH 2/3] fix error and add test --- ante/gov_vote_ante.go | 4 +-- ante/gov_vote_ante_authz_test.go | 62 ++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 ante/gov_vote_ante_authz_test.go diff --git a/ante/gov_vote_ante.go b/ante/gov_vote_ante.go index 11c5fe15..cc87a195 100644 --- a/ante/gov_vote_ante.go +++ b/ante/gov_vote_ante.go @@ -85,8 +85,8 @@ func (g GovVoteDecorator) validAuthz(ctx context.Context, execMsg *authz.MsgExec return errorsmod.Wrap(atomoneerrors.ErrUnauthorized, "cannot unmarshal authz exec msgs") } // Reject nested MsgExec to prevent bypassing stake checks - if _, ok := innerMsg.(*authz.MsgExec); ok { - return g.validAuthz(ctx, execMsg) + if msg, ok := innerMsg.(*authz.MsgExec); ok { + return g.validAuthz(ctx, msg) } if err := g.validMsg(ctx, innerMsg); err != nil { return err diff --git a/ante/gov_vote_ante_authz_test.go b/ante/gov_vote_ante_authz_test.go new file mode 100644 index 00000000..f021b395 --- /dev/null +++ b/ante/gov_vote_ante_authz_test.go @@ -0,0 +1,62 @@ +package ante_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "cosmossdk.io/math" + tmproto "github.com/cometbft/cometbft/proto/tendermint/types" + + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/authz" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + + "github.com/atomone-hub/atomone/ante" + "github.com/atomone-hub/atomone/app/helpers" + govv1beta1 "github.com/atomone-hub/atomone/x/gov/types/v1beta1" +) + +func TestVoteSpamDecoratorAuthz(t *testing.T) { + atomoneApp := helpers.Setup(t) + ctx := atomoneApp.NewUncachedContext(true, tmproto.Header{}) + decorator := ante.NewGovVoteDecorator(atomoneApp.AppCodec(), atomoneApp.StakingKeeper) + stakingKeeper := atomoneApp.StakingKeeper + + // Get validator + valAddrs, err := stakingKeeper.GetAllValidators(ctx) + require.NoError(t, err) + valAddr1, err := stakingKeeper.ValidatorAddressCodec().StringToBytes(valAddrs[0].OperatorAddress) + require.NoError(t, err) + + // Get delegator (this account was created during setup) + delegator, err := atomoneApp.AccountKeeper.Accounts.Indexes.Number.MatchExact(ctx, 0) + require.NoError(t, err) + + // Create another account for grantee + granteeAddr := sdk.AccAddress(ed25519.GenPrivKeyFromSecret([]byte{uint8(14)}).PubKey().Address()) + + // Delegate 1 atone so delegator has enough stake to vote + val, err := stakingKeeper.GetValidator(ctx, valAddr1) + require.NoError(t, err) + _, err = stakingKeeper.Delegate(ctx, delegator, math.NewInt(1000000), stakingtypes.Unbonded, val, true) + require.NoError(t, err) + + // Create inner vote message + innerMsg := govv1beta1.NewMsgVote( + nil, // nil addr + 0, + govv1beta1.OptionYes, + ) + + // Wrap in authz.MsgExec + execMsg := authz.NewMsgExec(granteeAddr, []sdk.Msg{innerMsg}) + err = decorator.ValidateVoteMsgs(ctx, []sdk.Msg{&execMsg}) + require.Error(t, err) + + // Create nested authz.MsgExec + nestedExecMsg := authz.NewMsgExec(granteeAddr, []sdk.Msg{&execMsg}) + err = decorator.ValidateVoteMsgs(ctx, []sdk.Msg{&nestedExecMsg}) + require.Error(t, err) +} From 9e280df77362b762c862d54cd6f028a31c18954b Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 18 Mar 2026 11:26:00 +0100 Subject: [PATCH 3/3] fix --- ante/gov_vote_ante.go | 5 ++++- ante/gov_vote_ante_authz_test.go | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/ante/gov_vote_ante.go b/ante/gov_vote_ante.go index cc87a195..53afe913 100644 --- a/ante/gov_vote_ante.go +++ b/ante/gov_vote_ante.go @@ -86,7 +86,10 @@ func (g GovVoteDecorator) validAuthz(ctx context.Context, execMsg *authz.MsgExec } // Reject nested MsgExec to prevent bypassing stake checks if msg, ok := innerMsg.(*authz.MsgExec); ok { - return g.validAuthz(ctx, msg) + if err := g.validAuthz(ctx, msg); err != nil { + return err + } + continue } if err := g.validMsg(ctx, innerMsg); err != nil { return err diff --git a/ante/gov_vote_ante_authz_test.go b/ante/gov_vote_ante_authz_test.go index f021b395..9743746e 100644 --- a/ante/gov_vote_ante_authz_test.go +++ b/ante/gov_vote_ante_authz_test.go @@ -59,4 +59,15 @@ func TestVoteSpamDecoratorAuthz(t *testing.T) { nestedExecMsg := authz.NewMsgExec(granteeAddr, []sdk.Msg{&execMsg}) err = decorator.ValidateVoteMsgs(ctx, []sdk.Msg{&nestedExecMsg}) require.Error(t, err) + + // Double nested authz.MsgExec + innerGoodMsg := govv1beta1.NewMsgVote( + delegator, + 0, + govv1beta1.OptionYes, + ) + execGoodMsg := authz.NewMsgExec(granteeAddr, []sdk.Msg{innerGoodMsg}) + nestedExecMsg = authz.NewMsgExec(granteeAddr, []sdk.Msg{&execGoodMsg, &execMsg}) + err = decorator.ValidateVoteMsgs(ctx, []sdk.Msg{&nestedExecMsg}) + require.Error(t, err) }