From 20785afeb70b76afdde4957b6bcf9c6adc9b2cb7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?=
 <31522760+fedekunze@users.noreply.github.com>
Date: Tue, 21 Sep 2021 12:29:59 +0200
Subject: [PATCH] evm: check tx cost not negative (#578)

---
 x/evm/keeper/utils.go      | 14 ++++++++++++--
 x/evm/keeper/utils_test.go | 13 ++++++++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/x/evm/keeper/utils.go b/x/evm/keeper/utils.go
index 807458dc..6dc69cbf 100644
--- a/x/evm/keeper/utils.go
+++ b/x/evm/keeper/utils.go
@@ -72,7 +72,8 @@ func DeductTxCostsFromUserBalance(
 	return fees, nil
 }
 
-// CheckSenderBalance validates sender has enough funds to pay for tx cost
+// CheckSenderBalance validates that the tx cost value is positive and that the
+// sender has enough funds to pay for the fees and value of the transaction.
 func CheckSenderBalance(
 	ctx sdk.Context,
 	bankKeeper evmtypes.BankKeeper,
@@ -83,7 +84,16 @@ func CheckSenderBalance(
 	balance := bankKeeper.GetBalance(ctx, sender, denom)
 	cost := txData.Cost()
 
-	if balance.Amount.BigInt().Cmp(cost) < 0 {
+	if cost.Sign() < 0 {
+		return stacktrace.Propagate(
+			sdkerrors.Wrapf(
+				sdkerrors.ErrInvalidCoins,
+				"tx cost (%s%s) is negative and invalid", cost, denom,
+			),
+			"tx cost amount should never be negative")
+	}
+
+	if balance.IsNegative() || balance.Amount.BigInt().Cmp(cost) < 0 {
 		return stacktrace.Propagate(
 			sdkerrors.Wrapf(
 				sdkerrors.ErrInsufficientFunds,
diff --git a/x/evm/keeper/utils_test.go b/x/evm/keeper/utils_test.go
index 456bf2d1..a1c765ba 100644
--- a/x/evm/keeper/utils_test.go
+++ b/x/evm/keeper/utils_test.go
@@ -13,9 +13,10 @@ import (
 func (suite *KeeperTestSuite) TestCheckSenderBalance() {
 	hundredInt := sdk.NewInt(100)
 	zeroInt := sdk.ZeroInt()
-	oneInt := sdk.NewInt(1)
+	oneInt := sdk.OneInt()
 	fiveInt := sdk.NewInt(5)
 	fiftyInt := sdk.NewInt(50)
+	negInt := sdk.NewInt(-10)
 
 	testCases := []struct {
 		name       string
@@ -47,6 +48,16 @@ func (suite *KeeperTestSuite) TestCheckSenderBalance() {
 			accessList: &ethtypes.AccessList{},
 			expectPass: true,
 		},
+		{
+			name:       "negative cost",
+			to:         suite.address.String(),
+			gasLimit:   1,
+			gasPrice:   &oneInt,
+			cost:       &negInt,
+			from:       suite.address.String(),
+			accessList: &ethtypes.AccessList{},
+			expectPass: false,
+		},
 		{
 			name:       "Higher gas limit, not enough balance",
 			to:         suite.address.String(),