Fix/authz

app/ante.go

@@ -69,6 +69,15 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
 		wasmkeeper.NewLimitSimulationGasDecorator(options.NodeConfig.SimulationGasLimit), // after setup context to enforce limits early
 		wasmkeeper.NewCountTXDecorator(options.TXCounterStoreService),
 		circuitante.NewCircuitBreakerDecorator(options.CircuitKeeper),
+		// SECURITY: Prevent dangerous nested messages in authz execution
+		NewAuthzLimiterDecorator([]string{
+			"/cosmwasm.wasm.v1.MsgExecuteContract",
+			"/cosmwasm.wasm.v1.MsgInstantiateContract",
+			"/cosmwasm.wasm.v1.MsgInstantiateContract2",
+			"/cosmwasm.wasm.v1.MsgMigrateContract",
+			"/cosmwasm.wasm.v1.MsgUpdateAdmin",
+			"/cosmwasm.wasm.v1.MsgClearAdmin",
+		}),
 		ante.NewExtensionOptionsDecorator(options.ExtensionOptionChecker),
 		// this changes the minGasFees,
 		// and must occur before gas fee checks

app/authz_ante.go

@@ -0,0 +1,63 @@
+package app
+
+import (
+	errorsmod "cosmossdk.io/errors"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/x/authz"
+)
+
+// AuthzLimiterDecorator prevents dangerous message types from being executed via authz
+type AuthzLimiterDecorator struct {
+	restrictedMessages map[string]bool
+}
+
+// NewAuthzLimiterDecorator creates a new AuthzLimiterDecorator with specified restricted message types
+func NewAuthzLimiterDecorator(restrictedMsgTypes []string) AuthzLimiterDecorator {
+	restricted := make(map[string]bool)
+	for _, msgType := range restrictedMsgTypes {
+		restricted[msgType] = true
+	}
+	return AuthzLimiterDecorator{
+		restrictedMessages: restricted,
+	}
+}
+
+// AnteHandle implements the AnteDecorator interface
+func (ald AuthzLimiterDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
+	// Check all messages in the transaction
+	for _, msg := range tx.GetMsgs() {
+		// If this is an authz MsgExec, inspect nested messages
+		if authzMsg, ok := msg.(*authz.MsgExec); ok {
+			if err := ald.ValidateAuthzMessages(authzMsg); err != nil {
+				return ctx, err
+			}
+		}
+	}
+
+	return next(ctx, tx, simulate)
+}
+
+// ValidateAuthzMessages checks if any nested messages in authz.MsgExec are restricted
+func (ald AuthzLimiterDecorator) ValidateAuthzMessages(authzMsg *authz.MsgExec) error {
+	// Extract nested messages from authz
+	nestedMsgs, err := authzMsg.GetMessages()
+	if err != nil {
+		return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "failed to get messages from authz: %v", err)
+	}
+
+	// Check each nested message against restricted types
+	for _, nestedMsg := range nestedMsgs {
+		msgType := sdk.MsgTypeURL(nestedMsg)
+		if ald.restrictedMessages[msgType] {
+			return errorsmod.Wrapf(
+				sdkerrors.ErrUnauthorized,
+				"message type %s is not allowed in authz execution due to security restrictions",
+				msgType,
+			)
+		}
+	}
+
+	return nil
+}

app/authz_ante_test.go

@@ -0,0 +1,98 @@
+package app
+
+import (
+	"testing"
+
+	wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cosmos/cosmos-sdk/codec/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/authz"
+)
+
+func TestAuthzLimiterDecorator_ValidateAuthzMessages(t *testing.T) {
+	// Create the decorator with restricted message types
+	decorator := NewAuthzLimiterDecorator([]string{
+		"/cosmwasm.wasm.v1.MsgExecuteContract",
+		"/cosmwasm.wasm.v1.MsgInstantiateContract",
+	})
+
+	// Test 1: Non-restricted message should pass
+	allowedMsg := &authz.MsgGrant{}
+	authzWithAllowed := createAuthzMsg(t, allowedMsg)
+
+	err := decorator.ValidateAuthzMessages(authzWithAllowed)
+	require.NoError(t, err)
+
+	// Test 2: Restricted MsgExecuteContract should be blocked
+	dangerousMsg := &wasmtypes.MsgExecuteContract{
+		Sender:   "xion1attacker",
+		Contract: "xion1malicious",
+		Msg:      []byte(`{"infinite_loop": {"iterations": 999999999}}`),
+		Funds:    nil,
+	}
+
+	authzWithDangerous := createAuthzMsg(t, dangerousMsg)
+	err = decorator.ValidateAuthzMessages(authzWithDangerous)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "/cosmwasm.wasm.v1.MsgExecuteContract")
+	require.Contains(t, err.Error(), "not allowed in authz execution")
+
+	// Test 3: Restricted MsgInstantiateContract should be blocked
+	dangerousMsg2 := &wasmtypes.MsgInstantiateContract{
+		Sender: "xion1attacker",
+		Admin:  "xion1attacker",
+		CodeID: 1,
+		Label:  "malicious",
+		Msg:    []byte("{}"),
+		Funds:  nil,
+	}
+
+	authzWithDangerous2 := createAuthzMsg(t, dangerousMsg2)
+	err = decorator.ValidateAuthzMessages(authzWithDangerous2)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "/cosmwasm.wasm.v1.MsgInstantiateContract")
+	require.Contains(t, err.Error(), "not allowed in authz execution")
+}
+
+func TestAuthzLimiterDecorator_EmptyRestrictedList(t *testing.T) {
+	// Decorator with no restricted messages should allow everything
+	decorator := NewAuthzLimiterDecorator([]string{})
+
+	dangerousMsg := &wasmtypes.MsgExecuteContract{
+		Sender:   "xion1sender",
+		Contract: "xion1contract",
+		Msg:      []byte("{}"),
+		Funds:    nil,
+	}
+
+	authzMsg := createAuthzMsg(t, dangerousMsg)
+	err := decorator.ValidateAuthzMessages(authzMsg)
+	require.NoError(t, err)
+}
+
+func TestAuthzLimiterDecorator_NewCreation(t *testing.T) {
+	restrictedTypes := []string{
+		"/cosmwasm.wasm.v1.MsgExecuteContract",
+		"/cosmwasm.wasm.v1.MsgInstantiateContract",
+	}
+
+	decorator := NewAuthzLimiterDecorator(restrictedTypes)
+
+	// Verify the decorator has the correct restricted messages
+	require.True(t, decorator.restrictedMessages["/cosmwasm.wasm.v1.MsgExecuteContract"])
+	require.True(t, decorator.restrictedMessages["/cosmwasm.wasm.v1.MsgInstantiateContract"])
+	require.False(t, decorator.restrictedMessages["/cosmos.authz.v1beta1.MsgGrant"])
+}
+
+// Helper function
+func createAuthzMsg(t *testing.T, nestedMsg sdk.Msg) *authz.MsgExec {
+	anyMsg, err := types.NewAnyWithValue(nestedMsg)
+	require.NoError(t, err)
+
+	return &authz.MsgExec{
+		Grantee: "xion1grantee",
+		Msgs:    []*types.Any{anyMsg},
+	}
+}

app/authz_integration_test.go

@@ -0,0 +1,161 @@
+package app_test
+
+import (
+	"testing"
+	"time"
+
+	wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cosmos/cosmos-sdk/codec/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/authz"
+
+	"github.com/burnt-labs/xion/app"
+)
+
+// TestAuthzBypassVulnerabilityPrevention demonstrates that the AuthzLimiterDecorator
+// successfully prevents the authz bypass attack described in the security report
+func TestAuthzBypassVulnerabilityPrevention(t *testing.T) {
+	// Create a malicious MsgExecuteContract that would cause network shutdown
+	maliciousContract := &wasmtypes.MsgExecuteContract{
+		Sender:   sdk.AccAddress([]byte("attacker")).String(),
+		Contract: sdk.AccAddress([]byte("malicious_contract")).String(),
+		Msg:      []byte(`{"infinite_loop": {"iterations": 999999999}}`),
+		Funds:    nil,
+	}
+
+	// Wrap it in an authz MsgExec to bypass ante handlers (the vulnerability)
+	anyMsg, err := types.NewAnyWithValue(maliciousContract)
+	require.NoError(t, err)
+
+	authzMsg := &authz.MsgExec{
+		Grantee: "xion1attacker",
+		Msgs:    []*types.Any{anyMsg},
+	}
+
+	// Create the AuthzLimiterDecorator with the same restrictions as in production
+	decorator := app.NewAuthzLimiterDecorator([]string{
+		"/cosmwasm.wasm.v1.MsgExecuteContract",
+		"/cosmwasm.wasm.v1.MsgInstantiateContract",
+		"/cosmwasm.wasm.v1.MsgInstantiateContract2",
+		"/cosmwasm.wasm.v1.MsgMigrateContract",
+		"/cosmwasm.wasm.v1.MsgUpdateAdmin",
+		"/cosmwasm.wasm.v1.MsgClearAdmin",
+	})
+
+	// Test that the attack is blocked
+	err = decorator.ValidateAuthzMessages(authzMsg)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "/cosmwasm.wasm.v1.MsgExecuteContract")
+	require.Contains(t, err.Error(), "not allowed in authz execution")
+
+	// Verify that the error message indicates security restrictions
+	require.Contains(t, err.Error(), "security restrictions")
+}
+
+// TestAuthzLimiterAllowsLegitimateMessages ensures we don't break normal authz functionality
+func TestAuthzLimiterAllowsLegitimateMessages(t *testing.T) {
+	// Create a legitimate authz message (MsgGrant is safe)
+	grantAuth, err := types.NewAnyWithValue(&authz.GenericAuthorization{
+		Msg: "/cosmos.bank.v1beta1.MsgSend",
+	})
+	require.NoError(t, err)
+
+	expiration := time.Now().Add(24 * time.Hour)
+	legitimateMsg := &authz.MsgGrant{
+		Granter: "xion1granter",
+		Grantee: "xion1grantee",
+		Grant: authz.Grant{
+			Authorization: grantAuth,
+			Expiration:    &expiration,
+		},
+	}
+
+	// Wrap it in authz MsgExec
+	anyMsg, err := types.NewAnyWithValue(legitimateMsg)
+	require.NoError(t, err)
+
+	authzMsg := &authz.MsgExec{
+		Grantee: "xion1grantee",
+		Msgs:    []*types.Any{anyMsg},
+	}
+
+	// Create the decorator
+	decorator := app.NewAuthzLimiterDecorator([]string{
+		"/cosmwasm.wasm.v1.MsgExecuteContract",
+		"/cosmwasm.wasm.v1.MsgInstantiateContract",
+	})
+
+	// Test that legitimate messages are allowed
+	err = decorator.ValidateAuthzMessages(authzMsg)
+	require.NoError(t, err)
+}
+
+// TestMultipleRestrictedMessages ensures all dangerous message types are blocked
+func TestMultipleRestrictedMessages(t *testing.T) {
+	restrictedMsgTypes := []sdk.Msg{
+		&wasmtypes.MsgExecuteContract{
+			Sender:   "xion1attacker",
+			Contract: "xion1contract",
+			Msg:      []byte("{}"),
+		},
+		&wasmtypes.MsgInstantiateContract{
+			Sender: "xion1attacker",
+			Admin:  "xion1attacker",
+			CodeID: 1,
+			Label:  "malicious",
+			Msg:    []byte("{}"),
+		},
+		&wasmtypes.MsgInstantiateContract2{
+			Sender: "xion1attacker",
+			Admin:  "xion1attacker",
+			CodeID: 1,
+			Label:  "malicious",
+			Salt:   []byte("salt"),
+			Msg:    []byte("{}"),
+		},
+		&wasmtypes.MsgMigrateContract{
+			Sender:   "xion1attacker",
+			Contract: "xion1contract",
+			CodeID:   2,
+			Msg:      []byte("{}"),
+		},
+		&wasmtypes.MsgUpdateAdmin{
+			Sender:   "xion1attacker",
+			NewAdmin: "xion1newadmin",
+			Contract: "xion1contract",
+		},
+		&wasmtypes.MsgClearAdmin{
+			Sender:   "xion1attacker",
+			Contract: "xion1contract",
+		},
+	}
+
+	decorator := app.NewAuthzLimiterDecorator([]string{
+		"/cosmwasm.wasm.v1.MsgExecuteContract",
+		"/cosmwasm.wasm.v1.MsgInstantiateContract",
+		"/cosmwasm.wasm.v1.MsgInstantiateContract2",
+		"/cosmwasm.wasm.v1.MsgMigrateContract",
+		"/cosmwasm.wasm.v1.MsgUpdateAdmin",
+		"/cosmwasm.wasm.v1.MsgClearAdmin",
+	})
+
+	for i, msg := range restrictedMsgTypes {
+		t.Run(sdk.MsgTypeURL(msg), func(t *testing.T) {
+			// Wrap in authz
+			anyMsg, err := types.NewAnyWithValue(msg)
+			require.NoError(t, err)
+
+			authzMsg := &authz.MsgExec{
+				Grantee: "xion1attacker",
+				Msgs:    []*types.Any{anyMsg},
+			}
+
+			// Should be blocked
+			err = decorator.ValidateAuthzMessages(authzMsg)
+			require.Error(t, err, "Message type %d should be blocked", i)
+			require.Contains(t, err.Error(), "not allowed in authz execution")
+		})
+	}
+}