Block eth msgs from authz (#1241)

* add decorator to block msgs in authz

* add to antehandler

* prevent vesting msgs skirting block via authz

* handle edge case of nested exec msgs

* test case to ensure msgs only blocked inside authz

* add app integration test

* tidy up error msg

app/ante/ante.go

@@ -8,6 +8,7 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
 	authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
+	vesting "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
 	ibcante "github.com/cosmos/ibc-go/v3/modules/core/ante"
 	ibckeeper "github.com/cosmos/ibc-go/v3/modules/core/keeper"
 	tmlog "github.com/tendermint/tendermint/libs/log"
@@ -104,6 +105,10 @@ func newCosmosAnteHandler(options HandlerOptions) sdk.AnteHandler {
 	decorators = append(decorators,
 		authante.NewMempoolFeeDecorator(),
 		NewVestingAccountDecorator(),
+		NewAuthzLimiterDecorator(
+			sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
+			sdk.MsgTypeURL(&vesting.MsgCreateVestingAccount{}),
+		),
 		authante.NewValidateBasicDecorator(),
 		authante.NewTxTimeoutHeightDecorator(),
 		authante.NewValidateMemoDecorator(options.AccountKeeper),

app/ante/ante_test.go

@@ -1,6 +1,7 @@
 package ante_test
 
 import (
+	"os"
 	"testing"
 	"time"
 
@@ -8,18 +9,27 @@ import (
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	"github.com/cosmos/cosmos-sdk/simapp/helpers"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
+	authz "github.com/cosmos/cosmos-sdk/x/authz"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"github.com/stretchr/testify/require"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/libs/log"
 	tmdb "github.com/tendermint/tm-db"
+	evmtypes "github.com/tharsis/ethermint/x/evm/types"
 
 	"github.com/kava-labs/kava/app"
 	bep3types "github.com/kava-labs/kava/x/bep3/types"
 	pricefeedtypes "github.com/kava-labs/kava/x/pricefeed/types"
 )
 
-func TestAppAnteHandler(t *testing.T) {
+func TestMain(m *testing.M) {
+	app.SetSDKConfig()
+	os.Exit(m.Run())
+}
+
+func TestAppAnteHandler_AuthorizedMempool(t *testing.T) {
 	testPrivKeys, testAddresses := app.GeneratePrivKeyAddressPairs(10)
 	unauthed := testAddresses[0:2]
 	unauthedKeys := testPrivKeys[0:2]
@@ -177,3 +187,80 @@ func newBep3GenStateMulti(cdc codec.JSONCodec, deputyAddress sdk.AccAddress) app
 	}
 	return app.GenesisState{bep3types.ModuleName: cdc.MustMarshalJSON(&bep3Genesis)}
 }
+
+func TestAppAnteHandler_RejectMsgsInAuthz(t *testing.T) {
+	testPrivKeys, testAddresses := app.GeneratePrivKeyAddressPairs(10)
+
+	newMsgGrant := func(msgTypeUrl string) *authz.MsgGrant {
+		msg, err := authz.NewMsgGrant(
+			testAddresses[0],
+			testAddresses[1],
+			authz.NewGenericAuthorization(msgTypeUrl),
+			time.Date(9000, 1, 1, 0, 0, 0, 0, time.UTC),
+		)
+		if err != nil {
+			panic(err)
+		}
+		return msg
+	}
+
+	chainID := "kavatest_1-1"
+	encodingConfig := app.MakeEncodingConfig()
+
+	testcases := []struct {
+		name         string
+		msg          sdk.Msg
+		expectedCode uint32
+	}{
+		{
+			name:         "MsgEthereumTx is blocked",
+			msg:          newMsgGrant(sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{})),
+			expectedCode: sdkerrors.ErrUnauthorized.ABCICode(),
+		},
+		{
+			name:         "MsgCreateVestingAccount is blocked",
+			msg:          newMsgGrant(sdk.MsgTypeURL(&vestingtypes.MsgCreateVestingAccount{})),
+			expectedCode: sdkerrors.ErrUnauthorized.ABCICode(),
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			tApp := app.NewTestApp()
+
+			tApp = tApp.InitializeFromGenesisStatesWithTimeAndChainID(
+				time.Date(1998, 1, 1, 0, 0, 0, 0, time.UTC),
+				chainID,
+			)
+
+			stdTx, err := helpers.GenTx(
+				encodingConfig.TxConfig,
+				[]sdk.Msg{tc.msg},
+				sdk.NewCoins(), // no fee
+				helpers.DefaultGenTxGas,
+				chainID,
+				[]uint64{0},
+				[]uint64{0},
+				testPrivKeys[0],
+			)
+			require.NoError(t, err)
+			txBytes, err := encodingConfig.TxConfig.TxEncoder()(stdTx)
+			require.NoError(t, err)
+
+			resCheckTx := tApp.CheckTx(
+				abci.RequestCheckTx{
+					Tx:   txBytes,
+					Type: abci.CheckTxType_New,
+				},
+			)
+			require.Equal(t, resCheckTx.Code, tc.expectedCode, resCheckTx.Log)
+
+			resDeliverTx := tApp.DeliverTx(
+				abci.RequestDeliverTx{
+					Tx: txBytes,
+				},
+			)
+			require.Equal(t, resDeliverTx.Code, tc.expectedCode, resDeliverTx.Log)
+		})
+	}
+}

app/ante/authz.go

@@ -0,0 +1,79 @@
+package ante
+
+import (
+	"fmt"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/x/authz"
+)
+
+// AuthzLimiterDecorator blocks certain msg types from being granted or executed within authz.
+type AuthzLimiterDecorator struct {
+	// disabledMsgTypes is the type urls of the msgs to block.
+	disabledMsgTypes []string
+}
+
+// NewAuthzLimiterDecorator creates a decorator to block certain msg types from being granted or executed within authz.
+func NewAuthzLimiterDecorator(disabledMsgTypes ...string) AuthzLimiterDecorator {
+	return AuthzLimiterDecorator{
+		disabledMsgTypes: disabledMsgTypes,
+	}
+}
+
+func (ald AuthzLimiterDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
+	err = ald.checkForDisabledMsg(tx.GetMsgs(), true)
+	if err != nil {
+		return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%v", err)
+	}
+	return next(ctx, tx, simulate)
+}
+
+// checkForDisabledMsg iterates through the msgs and returns an error if it finds any unauthorized msgs.
+//
+// When searchOnlyInAuthzMsgs is enabled, only authz MsgGrant and MsgExec are blocked, if they contain unauthorized msg types.
+// Otherwise any msg matching the disabled types are blocked, regardless of being in an authz msg or not.
+//
+// This method is recursive as MsgExec's can wrap other MsgExecs.
+func (ald AuthzLimiterDecorator) checkForDisabledMsg(msgs []sdk.Msg, searchOnlyInAuthzMsgs bool) error {
+	for _, msg := range msgs {
+		typeURL := sdk.MsgTypeURL(msg)
+		switch {
+		case !searchOnlyInAuthzMsgs && ald.isDisabled(typeURL):
+			return fmt.Errorf("found disabled msg type: %s", typeURL)
+
+		case typeURL == sdk.MsgTypeURL(&authz.MsgGrant{}):
+			m, ok := msg.(*authz.MsgGrant)
+			if !ok {
+				panic("unexpected msg type")
+			}
+			authorization := m.GetAuthorization()
+			if ald.isDisabled(authorization.MsgTypeURL()) {
+				return fmt.Errorf("found disabled msg type in MsgGrant: %s", authorization.MsgTypeURL())
+			}
+
+		case typeURL == sdk.MsgTypeURL(&authz.MsgExec{}):
+			m, ok := msg.(*authz.MsgExec)
+			if !ok {
+				panic("unexpected msg type")
+			}
+			innerMsgs, err := m.GetMessages()
+			if err != nil {
+				return err
+			}
+			if err := ald.checkForDisabledMsg(innerMsgs, false); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func (ald AuthzLimiterDecorator) isDisabled(msgTypeURL string) bool {
+	for _, disabledType := range ald.disabledMsgTypes {
+		if msgTypeURL == disabledType {
+			return true
+		}
+	}
+	return false
+}

app/ante/authz_test.go

@@ -0,0 +1,236 @@
+package ante_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/cosmos/cosmos-sdk/simapp/helpers"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/x/authz"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
+	"github.com/stretchr/testify/require"
+	evmtypes "github.com/tharsis/ethermint/x/evm/types"
+
+	"github.com/kava-labs/kava/app"
+	"github.com/kava-labs/kava/app/ante"
+)
+
+func newMsgGrant(granter sdk.AccAddress, grantee sdk.AccAddress, a authz.Authorization, expiration time.Time) *authz.MsgGrant {
+	msg, err := authz.NewMsgGrant(granter, grantee, a, expiration)
+	if err != nil {
+		panic(err)
+	}
+	return msg
+}
+
+func newMsgExec(grantee sdk.AccAddress, msgs []sdk.Msg) *authz.MsgExec {
+	msg := authz.NewMsgExec(grantee, msgs)
+	return &msg
+}
+
+func TestAuthzLimiterDecorator(t *testing.T) {
+	testPrivKeys, testAddresses := app.GeneratePrivKeyAddressPairs(5)
+	distantFuture := time.Date(9000, 1, 1, 0, 0, 0, 0, time.UTC)
+
+	validator := sdk.ValAddress(testAddresses[4])
+	stakingAuthDelegate, err := stakingtypes.NewStakeAuthorization([]sdk.ValAddress{validator}, nil, stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_DELEGATE, nil)
+	require.NoError(t, err)
+	stakingAuthUndelegate, err := stakingtypes.NewStakeAuthorization([]sdk.ValAddress{validator}, nil, stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_UNDELEGATE, nil)
+	require.NoError(t, err)
+
+	decorator := ante.NewAuthzLimiterDecorator(
+		sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
+		sdk.MsgTypeURL(&stakingtypes.MsgUndelegate{}),
+	)
+
+	testCases := []struct {
+		name        string
+		msgs        []sdk.Msg
+		checkTx     bool
+		expectedErr error
+	}{
+		{
+			name: "a non blocked msg is not blocked",
+			msgs: []sdk.Msg{
+				banktypes.NewMsgSend(
+					testAddresses[0],
+					testAddresses[1],
+					sdk.NewCoins(sdk.NewInt64Coin("ukava", 100e6)),
+				),
+			},
+			checkTx: false,
+		},
+		{
+			name: "a blocked msg is not blocked when not wrapped in MsgExec",
+			msgs: []sdk.Msg{
+				&evmtypes.MsgEthereumTx{},
+			},
+			checkTx: false,
+		},
+		{
+			name: "when a MsgGrant contains a non blocked msg, it passes",
+			msgs: []sdk.Msg{
+				newMsgGrant(
+					testAddresses[0],
+					testAddresses[1],
+					authz.NewGenericAuthorization(sdk.MsgTypeURL(&banktypes.MsgSend{})),
+					distantFuture,
+				),
+			},
+			checkTx: false,
+		},
+		{
+			name: "when a MsgGrant contains a non blocked msg, it passes",
+			msgs: []sdk.Msg{
+				newMsgGrant(
+					testAddresses[0],
+					testAddresses[1],
+					stakingAuthDelegate,
+					distantFuture,
+				),
+			},
+			checkTx: false,
+		},
+		{
+			name: "when a MsgGrant contains a blocked msg, it is blocked",
+			msgs: []sdk.Msg{
+				newMsgGrant(
+					testAddresses[0],
+					testAddresses[1],
+					authz.NewGenericAuthorization(sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{})),
+					distantFuture,
+				),
+			},
+			checkTx:     false,
+			expectedErr: sdkerrors.ErrUnauthorized,
+		},
+		{
+			name: "when a MsgGrant contains a blocked msg, it is blocked",
+			msgs: []sdk.Msg{
+				newMsgGrant(
+					testAddresses[0],
+					testAddresses[1],
+					stakingAuthUndelegate,
+					distantFuture,
+				),
+			},
+			checkTx:     false,
+			expectedErr: sdkerrors.ErrUnauthorized,
+		},
+		{
+			name: "when a MsgExec contains a non blocked msg, it passes",
+			msgs: []sdk.Msg{
+				newMsgExec(
+					testAddresses[1],
+					[]sdk.Msg{banktypes.NewMsgSend(
+						testAddresses[0],
+						testAddresses[3],
+						sdk.NewCoins(sdk.NewInt64Coin("ukava", 100e6)),
+					)}),
+			},
+			checkTx: false,
+		},
+		{
+			name: "when a MsgExec contains a blocked msg, it is blocked",
+			msgs: []sdk.Msg{
+				newMsgExec(
+					testAddresses[1],
+					[]sdk.Msg{
+						&evmtypes.MsgEthereumTx{},
+					},
+				),
+			},
+			checkTx:     false,
+			expectedErr: sdkerrors.ErrUnauthorized,
+		},
+		{
+			name: "blocked msg surrounded by valid msgs is still blocked",
+			msgs: []sdk.Msg{
+				newMsgGrant(
+					testAddresses[0],
+					testAddresses[1],
+					stakingAuthDelegate,
+					distantFuture,
+				),
+				newMsgExec(
+					testAddresses[1],
+					[]sdk.Msg{
+						banktypes.NewMsgSend(
+							testAddresses[0],
+							testAddresses[3],
+							sdk.NewCoins(sdk.NewInt64Coin("ukava", 100e6)),
+						),
+						&evmtypes.MsgEthereumTx{},
+					},
+				),
+			},
+			checkTx:     false,
+			expectedErr: sdkerrors.ErrUnauthorized,
+		},
+		{
+			name: "a nested MsgExec containing a blocked msg is still blocked",
+			msgs: []sdk.Msg{
+				newMsgExec(
+					testAddresses[1],
+					[]sdk.Msg{
+						newMsgExec(
+							testAddresses[2],
+							[]sdk.Msg{
+								&evmtypes.MsgEthereumTx{},
+							},
+						),
+					},
+				),
+			},
+			checkTx:     false,
+			expectedErr: sdkerrors.ErrUnauthorized,
+		},
+		{
+			name: "a nested MsgGrant containing a blocked msg is still blocked",
+			msgs: []sdk.Msg{
+				newMsgExec(
+					testAddresses[1],
+					[]sdk.Msg{
+						newMsgGrant(
+							testAddresses[0],
+							testAddresses[1],
+							authz.NewGenericAuthorization(sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{})),
+							distantFuture,
+						),
+					},
+				),
+			},
+			checkTx:     false,
+			expectedErr: sdkerrors.ErrUnauthorized,
+		},
+	}
+
+	txConfig := app.MakeEncodingConfig().TxConfig
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+
+			tx, err := helpers.GenTx(
+				txConfig,
+				tc.msgs,
+				sdk.NewCoins(),
+				helpers.DefaultGenTxGas,
+				"testing-chain-id",
+				[]uint64{0},
+				[]uint64{0},
+				testPrivKeys[0],
+			)
+			require.NoError(t, err)
+			mmd := MockAnteHandler{}
+			ctx := sdk.Context{}.WithIsCheckTx(tc.checkTx)
+			_, err = decorator.AnteHandle(ctx, tx, false, mmd.AnteHandle)
+			if tc.expectedErr != nil {
+				require.ErrorIs(t, err, tc.expectedErr)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}