Problem: improve permission checking in a few messages

CHANGELOG.md

@@ -11,6 +11,7 @@
 - [#1216](https://github.com/crypto-org-chain/cronos/pull/1216) Update ethermint to fix of avoid redundant parse chainID from gensis when start server.
 - [#1230](https://github.com/crypto-org-chain/cronos/pull/1230) Fix mem store in versiondb multistore.
 - [#1233](https://github.com/crypto-org-chain/cronos/pull/1233) Re-emit logs in callback contract.
+- [#1256](https://github.com/crypto-org-chain/cronos/pull/1256) Improve permission checkings for some messages.
 
 ### State Machine Breaking
 

app/app.go

@@ -144,7 +144,6 @@ import (
 
 	memiavlstore "github.com/crypto-org-chain/cronos/store"
 	memiavlrootmulti "github.com/crypto-org-chain/cronos/store/rootmulti"
-	"github.com/crypto-org-chain/cronos/v2/app/ante"
 	"github.com/crypto-org-chain/cronos/v2/x/cronos"
 	cronosclient "github.com/crypto-org-chain/cronos/v2/x/cronos/client"
 	cronoskeeper "github.com/crypto-org-chain/cronos/v2/x/cronos/keeper"
@@ -936,7 +935,7 @@ func (app *App) setAnteHandler(txConfig client.TxConfig, maxGasWanted uint64, bl
 		blockedMap[string(addr)] = struct{}{}
 	}
 	blockAddressDecorator := NewBlockAddressesDecorator(blockedMap)
-	evmOptions := evmante.HandlerOptions{
+	options := evmante.HandlerOptions{
 		AccountKeeper:          app.AccountKeeper,
 		BankKeeper:             app.BankKeeper,
 		EvmKeeper:              app.EvmKeeper,
@@ -954,12 +953,8 @@ func (app *App) setAnteHandler(txConfig client.TxConfig, maxGasWanted uint64, bl
 		},
 		ExtraDecorators: []sdk.AnteDecorator{blockAddressDecorator},
 	}
-	options := ante.HandlerOptions{
-		EvmOptions:   evmOptions,
-		CronosKeeper: app.CronosKeeper,
-	}
 
-	anteHandler, err := ante.NewAnteHandler(options)
+	anteHandler, err := evmante.NewAnteHandler(options)
 	if err != nil {
 		return err
 	}

x/cronos/keeper/msg_server.go

@@ -66,6 +66,12 @@
 // UpdateTokenMapping implements the grpc method
 func (k msgServer) UpdateTokenMapping(goCtx context.Context, msg *types.MsgUpdateTokenMapping) (*types.MsgUpdateTokenMappingResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
+
+	// check permission
+	if !k.Keeper.HasPermission(ctx, msg.GetSigners(), CanChangeTokenMapping) {
+		return nil, errors.Wrap(sdkerrors.ErrUnauthorized, "msg sender is not authorized")
+	}
+
 	// msg is already validated
 	if err := k.Keeper.RegisterOrUpdateTokenMapping(ctx, msg); err != nil {
 		return nil, err
@@ -76,6 +82,12 @@
 // TurnBridge implements the grpc method
 func (k msgServer) TurnBridge(goCtx context.Context, msg *types.MsgTurnBridge) (*types.MsgTurnBridgeResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
+
+	// check permission
+	if !k.Keeper.HasPermission(ctx, msg.GetSigners(), CanTurnBridge) {
+		return nil, errors.Wrap(sdkerrors.ErrUnauthorized, "msg sender is not authorized")
+	}
+
 	gravityParams := k.gravityKeeper.GetParams(ctx)
 	gravityParams.BridgeActive = msg.Enable
 	k.gravityKeeper.SetParams(ctx, gravityParams)
@@ -101,7 +113,7 @@
 	admin := k.Keeper.GetParams(ctx).CronosAdmin
 	// if admin is empty, no sender could be equal to it
 	if admin != msg.From {
-		return nil, errors.Wrap(sdkerrors.ErrInvalidAddress, "msg sender is authorized")
+		return nil, errors.Wrap(sdkerrors.ErrUnauthorized, "msg sender is not authorized")
 	}
 	acc, err := sdk.AccAddressFromBech32(msg.Address)
 	if err != nil {

x/cronos/keeper/permissions.go

@@ -28,13 +28,21 @@
 }
 
 // HasPermission check if an account has a specific permission. by default cronos admin has all permissions
-func (k Keeper) HasPermission(ctx sdk.Context, account sdk.AccAddress, permissionsToCheck uint64) bool {
+func (k Keeper) HasPermission(ctx sdk.Context, accounts []sdk.AccAddress, permissionsToCheck uint64) bool {
 	// case when no permission is needed
 	if permissionsToCheck == 0 {
 		return true
 	}
 	admin := k.GetParams(ctx).CronosAdmin
-	permission := k.GetPermissions(ctx, account)
-	mask := permission & permissionsToCheck
-	return (admin == account.String()) || (mask == permissionsToCheck)
+	for _, account := range accounts {
+		if admin == account.String() {
+			return true
+		}
+		permission := k.GetPermissions(ctx, account)
+		if permission&permissionsToCheck == permissionsToCheck {
+			return true
+		}
+	}
+
+	return false
 }

x/cronos/keeper/permissions_test.go

@@ -41,23 +41,23 @@ func (suite *KeeperTestSuite) TestHasPermissions() {
 	priv, err := ethsecp256k1.GenerateKey()
 	suite.Require().NoError(err)
 	address := common.BytesToAddress(priv.PubKey().Address().Bytes())
-	cosmosAddress := sdk.AccAddress(address.Bytes())
+	cosmosAddress := []sdk.AccAddress{sdk.AccAddress(address.Bytes())}
 
 	suite.Require().Equal(true, keeper.HasPermission(suite.ctx, cosmosAddress, 0))
 	suite.Require().Equal(true, keeper.HasPermission(suite.ctx, cosmosAddress, 0))
 
 	suite.Require().Equal(false, keeper.HasPermission(suite.ctx, cosmosAddress, CanChangeTokenMapping))
 	suite.Require().Equal(false, keeper.HasPermission(suite.ctx, cosmosAddress, CanTurnBridge))
 
-	keeper.SetPermissions(suite.ctx, cosmosAddress, CanChangeTokenMapping)
+	keeper.SetPermissions(suite.ctx, cosmosAddress[0], CanChangeTokenMapping)
 	suite.Require().Equal(true, keeper.HasPermission(suite.ctx, cosmosAddress, CanChangeTokenMapping))
 	suite.Require().Equal(false, keeper.HasPermission(suite.ctx, cosmosAddress, CanTurnBridge))
 
-	keeper.SetPermissions(suite.ctx, cosmosAddress, CanTurnBridge)
+	keeper.SetPermissions(suite.ctx, cosmosAddress[0], CanTurnBridge)
 	suite.Require().Equal(false, keeper.HasPermission(suite.ctx, cosmosAddress, CanChangeTokenMapping))
 	suite.Require().Equal(true, keeper.HasPermission(suite.ctx, cosmosAddress, CanTurnBridge))
 
-	keeper.SetPermissions(suite.ctx, cosmosAddress, All)
+	keeper.SetPermissions(suite.ctx, cosmosAddress[0], All)
 	suite.Require().Equal(true, keeper.HasPermission(suite.ctx, cosmosAddress, CanChangeTokenMapping))
 	suite.Require().Equal(true, keeper.HasPermission(suite.ctx, cosmosAddress, CanTurnBridge))
 }

x/cronos/simulation/operations.go

@@ -91,7 +91,7 @@
 		}
 
 		oper, ops, err := simulation.GenAndDeliverTxWithRandFees(txCtx)
-		if simAccount.Address.String() != cronosAdmin && errors.Is(err, errorsmod.Wrap(sdkerrors.ErrInvalidAddress, "msg sender is authorized")) {
+		if simAccount.Address.String() != cronosAdmin && errors.Is(err, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "msg sender is not authorized")) {
 			return simtypes.NoOpMsg(types.ModuleName, msg.Type(), "unauthorized tx should fail"), nil, nil
 		}
 		return oper, ops, err