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 <[email protected]>
celestiaorg · Jun 18, 2024 · 85eb1cb · 85eb1cb
1 parent 754eae9
commit 85eb1cb
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 5 deletions.
diff --git 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
@@ -7,13 +7,17 @@ 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"
 	version "github.com/tendermint/tendermint/proto/tendermint/version"
 )
 
 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")