Merge branch 'main' into ud/evm-event

CHANGELOG.md

@@ -52,6 +52,7 @@ Remove unnecessary argument in the `VerifyFee` function, which returns the token
 payment required based on the effective fee from the tx data. Improve
 documentation.
 - [#2125](https://github.com/NibiruChain/nibiru/pull/2125) - feat(evm-precompile):Emit EVM events created to reflect the ABCI events that occur outside the EVM to make sure that block explorers and indexers can find indexed ABCI event information. 
+- [#2129](https://github.com/NibiruChain/nibiru/pull/2129) - fix(evm): issue with infinite recursion in erc20 funtoken contracts
 
 #### Nibiru EVM | Before Audit 2 - 2024-12-06
 

x/evm/embeds/contracts/TestInfiniteRecursionERC20.sol

@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import "./IFunToken.sol";
+import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+
+contract TestInfiniteRecursionERC20 is ERC20 {
+    constructor(string memory name, string memory symbol, uint8 decimals_)
+    ERC20(name, symbol) {
+        _mint(msg.sender, 1000000 * 10**18);
+    }
+
+    function balanceOf(address who) public view virtual override returns (uint256) {
+        // recurse through funtoken.balance(who, address(this))
+        address(FUNTOKEN_PRECOMPILE_ADDRESS).staticcall(
+            abi.encodeWithSignature(
+                "balance(address,address)",
+                who,
+                address(this))
+        );
+        return 0;
+    }
+
+    function transfer(address to, uint256 amount) public override returns (bool) {
+        // recurse through funtoken sendToBank
+        FUNTOKEN_PRECOMPILE.sendToBank(
+            address(this),
+            amount,
+            "nibi1zaavvzxez0elundtn32qnk9lkm8kmcsz44g7xl" // does not matter, it's not reached
+        );
+        return true;
+    }
+
+    function attackBalance() public {
+        balanceOf(address(0));
+    }
+
+    function attackTransfer() public {
+        transfer(address(0), 1);
+    }
+}

x/evm/embeds/embeds.go

@@ -37,6 +37,8 @@ var (
 	testNativeSendThenPrecompileSendJson []byte
 	//go:embed artifacts/contracts/TestPrecompileSelfCallRevert.sol/TestPrecompileSelfCallRevert.json
 	testPrecompileSelfCallRevertJson []byte
+	//go:embed artifacts/contracts/TestInfiniteRecursionERC20.sol/TestInfiniteRecursionERC20.json
+	testInfiniteRecursionERC20Json []byte
 )
 
 var (
@@ -118,6 +120,12 @@ var (
 		Name:      "TestPrecompileSelfCallRevert.sol",
 		EmbedJSON: testPrecompileSelfCallRevertJson,
 	}
+	// SmartContract_TestInfiniteRecursionERC20 is a test contract
+	// which simulates malicious ERC20 behavior by adding infinite recursion in transfer() and balanceOf() functions
+	SmartContract_TestInfiniteRecursionERC20 = CompiledEvmContract{
+		Name:      "TestInfiniteRecursionERC20.sol",
+		EmbedJSON: testInfiniteRecursionERC20Json,
+	}
 )
 
 func init() {
@@ -132,6 +140,7 @@ func init() {
 	SmartContract_TestNativeSendThenPrecompileSendJson.MustLoad()
 	SmartContract_TestERC20TransferThenPrecompileSend.MustLoad()
 	SmartContract_TestPrecompileSelfCallRevert.MustLoad()
+	SmartContract_TestInfiniteRecursionERC20.MustLoad()
 }
 
 type CompiledEvmContract struct {

x/evm/embeds/embeds_test.go

@@ -19,5 +19,6 @@ func TestLoadContracts(t *testing.T) {
 		embeds.SmartContract_TestFunTokenPrecompileLocalGas.MustLoad()
 		embeds.SmartContract_TestNativeSendThenPrecompileSendJson.MustLoad()
 		embeds.SmartContract_TestERC20TransferThenPrecompileSend.MustLoad()
+		embeds.SmartContract_TestInfiniteRecursionERC20.MustLoad()
 	})
 }

x/evm/keeper/erc20.go

@@ -3,6 +3,7 @@ package keeper
 
 import (
 	"fmt"
+	"math"
 	"math/big"
 
 	"cosmossdk.io/errors"
@@ -26,6 +27,14 @@ const (
 	Erc20GasLimitExecute uint64 = 200_000
 )
 
+// getCallGas returns the gas limit for a call to an ERC20 contract following 63/64 rule (EIP-150)
+// protection against recursive calls ERC20 -> precompile -> ERC20.
+func getCallGasWithLimit(ctx sdk.Context, gasLimit uint64) uint64 {
+	availableGas := ctx.GasMeter().GasRemaining()
+	callGas := availableGas - uint64(math.Floor(float64(availableGas)/64))
+	return min(callGas, gasLimit)
+}
+
 // ERC20 returns a mutable reference to the keeper with an ERC20 contract ABI and
 // Go functions corresponding to contract calls in the ERC20 standard like "mint"
 // and "transfer" in the ERC20 standard.
@@ -60,7 +69,7 @@ func (e erc20Calls) Mint(
 	contract, from, to gethcommon.Address, amount *big.Int,
 	ctx sdk.Context,
 ) (evmResp *evm.MsgEthereumTxResponse, err error) {
-	return e.CallContract(ctx, e.ABI, from, &contract, true, Erc20GasLimitExecute, "mint", to, amount)
+	return e.CallContract(ctx, e.ABI, from, &contract, true, getCallGasWithLimit(ctx, Erc20GasLimitExecute), "mint", to, amount)
 }
 
 /*
@@ -82,7 +91,7 @@ func (e erc20Calls) Transfer(
 		return balanceIncrease, nil, errors.Wrap(err, "failed to retrieve recipient balance")
 	}
 
-	resp, err = e.CallContract(ctx, e.ABI, from, &contract, true, Erc20GasLimitExecute, "transfer", to, amount)
+	resp, err = e.CallContract(ctx, e.ABI, from, &contract, true, getCallGasWithLimit(ctx, Erc20GasLimitExecute), "transfer", to, amount)
 	if err != nil {
 		return balanceIncrease, nil, err
 	}
@@ -141,7 +150,7 @@ func (e erc20Calls) Burn(
 	contract, from gethcommon.Address, amount *big.Int,
 	ctx sdk.Context,
 ) (evmResp *evm.MsgEthereumTxResponse, err error) {
-	return e.CallContract(ctx, e.ABI, from, &contract, true, Erc20GasLimitExecute, "burn", amount)
+	return e.CallContract(ctx, e.ABI, from, &contract, true, getCallGasWithLimit(ctx, Erc20GasLimitExecute), "burn", amount)
 }
 
 func (k Keeper) LoadERC20Name(
@@ -174,7 +183,7 @@ func (k Keeper) LoadERC20String(
 		evm.EVM_MODULE_ADDRESS,
 		&erc20Contract,
 		false,
-		Erc20GasLimitQuery,
+		getCallGasWithLimit(ctx, Erc20GasLimitQuery),
 		methodName,
 	)
 	if err != nil {
@@ -202,7 +211,7 @@ func (k Keeper) loadERC20Uint8(
 		evm.EVM_MODULE_ADDRESS,
 		&erc20Contract,
 		false,
-		Erc20GasLimitQuery,
+		getCallGasWithLimit(ctx, Erc20GasLimitQuery),
 		methodName,
 	)
 	if err != nil {
@@ -232,7 +241,7 @@ func (k Keeper) LoadERC20BigInt(
 		evm.EVM_MODULE_ADDRESS,
 		&contract,
 		false,
-		Erc20GasLimitQuery,
+		getCallGasWithLimit(ctx, Erc20GasLimitQuery),
 		methodName,
 		args...,
 	)

x/evm/keeper/funtoken_from_erc20_test.go

@@ -385,6 +385,73 @@ func (s *FunTokenFromErc20Suite) TestFunTokenFromERC20MaliciousTransfer() {
 	s.Require().ErrorContains(err, "gas required exceeds allowance")
 }
 
+// TestFunTokenInfiniteRecursionERC20 creates a funtoken from a contract
+// with a malicious recursive balanceOf() and transfer() functions.
+func (s *FunTokenFromErc20Suite) TestFunTokenInfiniteRecursionERC20() {
+	deps := evmtest.NewTestDeps()
+
+	s.T().Log("Deploy InfiniteRecursionERC20")
+	metadata := keeper.ERC20Metadata{
+		Name:     "erc20name",
+		Symbol:   "TOKEN",
+		Decimals: 18,
+	}
+	deployResp, err := evmtest.DeployContract(
+		&deps, embeds.SmartContract_TestInfiniteRecursionERC20,
+		metadata.Name, metadata.Symbol, metadata.Decimals,
+	)
+	s.Require().NoError(err)
+
+	erc20Addr := eth.EIP55Addr{
+		Address: deployResp.ContractAddr,
+	}
+
+	s.T().Log("happy: CreateFunToken for ERC20 with infinite recursion")
+	s.Require().NoError(testapp.FundAccount(
+		deps.App.BankKeeper,
+		deps.Ctx,
+		deps.Sender.NibiruAddr,
+		deps.EvmKeeper.FeeForCreateFunToken(deps.Ctx),
+	))
+
+	_, err = deps.EvmKeeper.CreateFunToken(
+		sdk.WrapSDKContext(deps.Ctx),
+		&evm.MsgCreateFunToken{
+			FromErc20: &erc20Addr,
+			Sender:    deps.Sender.NibiruAddr.String(),
+		},
+	)
+	s.Require().NoError(err)
+
+	deps.ResetGasMeter()
+
+	s.T().Log("happy: call attackBalance()")
+	res, err := deps.EvmKeeper.CallContract(
+		deps.Ctx,
+		embeds.SmartContract_TestInfiniteRecursionERC20.ABI,
+		deps.Sender.EthAddr,
+		&erc20Addr.Address,
+		false,
+		10_000_000,
+		"attackBalance",
+	)
+	s.Require().NoError(err)
+	s.Require().NotNil(res)
+	s.Require().Empty(res.VmError)
+
+	s.T().Log("sad: call attackBalance()")
+	_, err = deps.EvmKeeper.CallContract(
+		deps.Ctx,
+		embeds.SmartContract_TestInfiniteRecursionERC20.ABI,
+		deps.Sender.EthAddr,
+		&erc20Addr.Address,
+		true,
+		10_000_000,
+		"attackTransfer",
+	)
+	s.Require().ErrorContains(err, "execution reverted")
+}
+
 type FunTokenFromErc20Suite struct {
 	suite.Suite
 }