Disallow negative value passed into internal EVM calls (sei-protocol#…

…1590)

* Disallow negative value passed into internal EVM calls

* Update defensive checks when calling EVM internally

x/evm/keeper/evm.go

@@ -7,10 +7,12 @@ import (
 	"math/big"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/core/vm"
+	"github.com/ethereum/go-ethereum/params"
 	"github.com/sei-protocol/sei-chain/utils"
 	"github.com/sei-protocol/sei-chain/x/evm/state"
 	"github.com/sei-protocol/sei-chain/x/evm/types"
@@ -65,16 +67,22 @@ func (k *Keeper) HandleInternalEVMDelegateCall(ctx sdk.Context, req *types.MsgIn
 }
 
 func (k *Keeper) CallEVM(ctx sdk.Context, from common.Address, to *common.Address, val *sdk.Int, data []byte) (retdata []byte, reterr error) {
+	if to == nil && len(data) > params.MaxInitCodeSize {
+		return nil, fmt.Errorf("%w: code size %v, limit %v", core.ErrMaxInitCodeSizeExceeded, len(data), params.MaxInitCodeSize)
+	}
+	value := utils.Big0
+	if val != nil {
+		if val.IsNegative() {
+			return nil, sdkerrors.ErrInvalidCoins
+		}
+		value = val.BigInt()
+	}
 	evm := types.GetCtxEVM(ctx)
 	if evm == nil {
 		// This call was not part of an existing StateTransition, so it should trigger one
 		executionCtx := ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 		stateDB := state.NewDBImpl(executionCtx, k, false)
 		gp := k.GetGasPool()
-		value := utils.Big0
-		if val != nil {
-			value = val.BigInt()
-		}
 		evmMsg := &core.Message{
 			Nonce:             stateDB.GetNonce(from), // replay attack is prevented by the AccountSequence number set on the CW transaction that triggered this call
 			GasLimit:          k.getEvmGasLimitFromCtx(ctx),

x/evm/keeper/evm_test.go

@@ -6,6 +6,7 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/params"
 	testkeeper "github.com/sei-protocol/sei-chain/testutil/keeper"
 	"github.com/sei-protocol/sei-chain/x/evm/artifacts/native"
 	"github.com/sei-protocol/sei-chain/x/evm/types"
@@ -115,3 +116,55 @@ func TestStaticCall(t *testing.T) {
 	require.Equal(t, 1, len(decoded))
 	require.Equal(t, big.NewInt(int64(2000)), decoded[0].(*big.Int))
 }
+
+func TestNegativeTransfer(t *testing.T) {
+	steal_amount := int64(1_000_000_000_000)
+
+	k := testkeeper.EVMTestApp.EvmKeeper
+	ctx := testkeeper.EVMTestApp.NewContext(false, tmtypes.Header{}).WithBlockHeight(2)
+	attackerAddr, attackerEvmAddr := testkeeper.MockAddressPair()
+	victimAddr, victimEvmAddr := testkeeper.MockAddressPair()
+
+	// associate addrs
+	k.SetAddressMapping(ctx, attackerAddr, attackerEvmAddr)
+	k.SetAddressMapping(ctx, victimAddr, victimEvmAddr)
+
+	// mint some funds to victim
+	amt := sdk.NewCoins(sdk.NewCoin(k.GetBaseDenom(ctx), sdk.NewInt(steal_amount)))
+	require.Nil(t, k.BankKeeper().MintCoins(ctx, types.ModuleName, sdk.NewCoins(sdk.NewCoin(k.GetBaseDenom(ctx), sdk.NewInt(steal_amount)))))
+	require.Nil(t, k.BankKeeper().SendCoinsFromModuleToAccount(ctx, types.ModuleName, victimAddr, amt))
+
+	// construct attack payload
+	val := sdk.NewInt(steal_amount).Mul(sdk.NewInt(steal_amount * -1))
+	req := &types.MsgInternalEVMCall{
+		Sender: attackerAddr.String(),
+		Data:   []byte{},
+		Value:  &val,
+		To:     victimEvmAddr.Hex(),
+	}
+
+	// pre verification
+	preAttackerBal := testkeeper.EVMTestApp.BankKeeper.GetBalance(ctx, attackerAddr, k.GetBaseDenom(ctx)).Amount.Int64()
+	preVictimBal := testkeeper.EVMTestApp.BankKeeper.GetBalance(ctx, victimAddr, k.GetBaseDenom(ctx)).Amount.Int64()
+	require.Zero(t, preAttackerBal)
+	require.Equal(t, steal_amount, preVictimBal)
+
+	_, err := k.HandleInternalEVMCall(ctx, req)
+	require.ErrorContains(t, err, "invalid coins")
+
+	// post verification
+	postAttackerBal := testkeeper.EVMTestApp.BankKeeper.GetBalance(ctx, attackerAddr, k.GetBaseDenom(ctx)).Amount.Int64()
+	postVictimBal := testkeeper.EVMTestApp.BankKeeper.GetBalance(ctx, victimAddr, k.GetBaseDenom(ctx)).Amount.Int64()
+	require.Zero(t, postAttackerBal)
+	require.Equal(t, steal_amount, postVictimBal)
+
+	zeroVal := sdk.NewInt(0)
+	req2 := &types.MsgInternalEVMCall{
+		Sender: attackerAddr.String(),
+		Data:   make([]byte, params.MaxInitCodeSize+1),
+		Value:  &zeroVal,
+	}
+
+	_, err = k.HandleInternalEVMCall(ctx, req2)
+	require.ErrorContains(t, err, "max initcode size exceeded")
+}