From 85eb1cb707448b29dd821b7a0f72fd652f6ce473 Mon Sep 17 00:00:00 2001 From: Rootul P Date: Tue, 18 Jun 2024 13:31:50 -0600 Subject: [PATCH] fix!: msg gatekeeper for authz messages (#3555) Resolves this [security advisory](https://github.com/celestiaorg/celestia-app/security/advisories/GHSA-j4w8-q5hh-6xf9). Thanks @Reecepbcups for the report and fix! Co-authored-by: Reece Williams --- app/ante/msg_gatekeeper.go | 27 ++++++++++++++++++++--- app/ante/msg_gatekeeper_test.go | 39 +++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/app/ante/msg_gatekeeper.go b/app/ante/msg_gatekeeper.go index a3824d1c08..9f4841448a 100644 --- a/app/ante/msg_gatekeeper.go +++ b/app/ante/msg_gatekeeper.go @@ -6,6 +6,7 @@ import ( "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/authz" ) var ( @@ -32,15 +33,35 @@ func (mgk MsgVersioningGateKeeper) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula if !exists { return ctx, sdkerrors.ErrNotSupported.Wrapf("app version %d is not supported", ctx.BlockHeader().Version.App) } - for _, msg := range tx.GetMsgs() { + + if err := mgk.hasInvalidMsg(ctx, acceptedMsgs, tx.GetMsgs()); err != nil { + return ctx, err + } + + return next(ctx, tx, simulate) +} + +func (mgk MsgVersioningGateKeeper) hasInvalidMsg(ctx sdk.Context, acceptedMsgs map[string]struct{}, msgs []sdk.Msg) error { + for _, msg := range msgs { + // Recursively check for invalid messages in nested authz messages. + if execMsg, ok := msg.(*authz.MsgExec); ok { + nestedMsgs, err := execMsg.GetMessages() + if err != nil { + return err + } + if err = mgk.hasInvalidMsg(ctx, acceptedMsgs, nestedMsgs); err != nil { + return err + } + } + msgTypeURL := sdk.MsgTypeURL(msg) _, exists := acceptedMsgs[msgTypeURL] if !exists { - return ctx, sdkerrors.ErrNotSupported.Wrapf("message type %s is not supported in version %d", msgTypeURL, ctx.BlockHeader().Version.App) + return sdkerrors.ErrNotSupported.Wrapf("message type %s is not supported in version %d", msgTypeURL, ctx.BlockHeader().Version.App) } } - return next(ctx, tx, simulate) + return nil } func (mgk MsgVersioningGateKeeper) IsAllowed(ctx context.Context, msgName string) (bool, error) { diff --git a/app/ante/msg_gatekeeper_test.go b/app/ante/msg_gatekeeper_test.go index 4492d1061a..6bbedb1571 100644 --- a/app/ante/msg_gatekeeper_test.go +++ b/app/ante/msg_gatekeeper_test.go @@ -7,6 +7,7 @@ import ( "github.com/celestiaorg/celestia-app/v2/app/ante" "github.com/celestiaorg/celestia-app/v2/app/encoding" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/authz" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/stretchr/testify/require" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" @@ -14,6 +15,9 @@ import ( ) func TestMsgGateKeeperAnteHandler(t *testing.T) { + nestedBankSend := authz.NewMsgExec(sdk.AccAddress{}, []sdk.Msg{&banktypes.MsgSend{}}) + nestedMultiSend := authz.NewMsgExec(sdk.AccAddress{}, []sdk.Msg{&banktypes.MsgMultiSend{}}) + // Define test cases tests := []struct { name string @@ -27,23 +31,42 @@ func TestMsgGateKeeperAnteHandler(t *testing.T) { acceptMsg: true, version: 1, }, + { + name: "Accept nested MsgSend", + msg: &nestedBankSend, + acceptMsg: true, + version: 1, + }, { name: "Reject MsgMultiSend", msg: &banktypes.MsgMultiSend{}, acceptMsg: false, version: 1, }, + { + name: "Reject nested MsgMultiSend", + msg: &nestedMultiSend, + acceptMsg: false, + version: 1, + }, { name: "Reject MsgSend with version 2", msg: &banktypes.MsgSend{}, acceptMsg: false, version: 2, }, + { + name: "Reject nested MsgSend with version 2", + msg: &nestedBankSend, + acceptMsg: false, + version: 2, + }, } msgGateKeeper := ante.NewMsgVersioningGateKeeper(map[uint64]map[string]struct{}{ 1: { - "/cosmos.bank.v1beta1.MsgSend": {}, + "/cosmos.bank.v1beta1.MsgSend": {}, + "/cosmos.authz.v1beta1.MsgExec": {}, }, 2: {}, }) @@ -56,7 +79,19 @@ func TestMsgGateKeeperAnteHandler(t *testing.T) { txBuilder := cdc.TxConfig.NewTxBuilder() require.NoError(t, txBuilder.SetMsgs(tc.msg)) _, err := anteHandler(ctx, txBuilder.GetTx(), false) - allowed, err2 := msgGateKeeper.IsAllowed(ctx, sdk.MsgTypeURL(tc.msg)) + + msg := tc.msg + if sdk.MsgTypeURL(msg) == "/cosmos.authz.v1beta1.MsgExec" { + execMsg, ok := msg.(*authz.MsgExec) + require.True(t, ok) + + nestedMsgs, err := execMsg.GetMessages() + require.NoError(t, err) + msg = nestedMsgs[0] + } + + allowed, err2 := msgGateKeeper.IsAllowed(ctx, sdk.MsgTypeURL(msg)) + require.NoError(t, err2) if tc.acceptMsg { require.NoError(t, err, "expected message to be accepted")