chore: merge audit-fixes into develop

CHANGELOG.md

@@ -44,6 +44,12 @@ Follow the [migration document](docs/migrations/v0.5.x_to_v0.6.0.md) for upgrade
 
 ### BUG FIXES
 
+- Re-apply the `validateApprovalEventDoesNotExist` guard (F-2026-18822) on the ERC20 -> coin
+  direction. v0.6.0 lifts that body out of `x/erc20/keeper/msg_server.go` into the new shared
+  `x/erc20/keeper/convert.go` (`ConvertERC20IntoCoinsForNativeToken`, also reached from the ICS20
+  precompile) and drops the call on the way, while still promising it in the doc comment. The guard
+  is restored in the position it held before the move, and the `delayed malicious contract` case in
+  `TestConvertERC20IntoCoinsForNativeToken` expects a rejection again.
 - Report the block base fee (instead of `0`) as the `gasPrice` of derived EVM transactions in
   `eth_getTransactionByHash` / `eth_getBlockByNumber`, and as their receipt `effectiveGasPrice`.
   Derived txs carry zero fee caps, so consumers that model burn as `base_fee * gas_used` — such as
@@ -60,6 +66,71 @@ Follow the [migration document](docs/migrations/v0.5.x_to_v0.6.0.md) for upgrade
 
 ### BUG FIXES
 
+- Honour the `gasCap` argument in `CallEVMWithData`. The parameter was accepted and then ignored -
+  `GasLimit` was hardcoded to `config.DefaultGasCap` - so callers passing a cap to sandbox EVM work
+  (the IBC callback keeper passes its remaining Cosmos gas) got a message that could burn up to 25M
+  gas regardless. `DefaultGasCap` remains the ceiling, so a caller can only narrow the limit. This
+  matches `DerivedEVMCallWithData` and the shape upstream cosmos/evm settled on. Also routes the
+  timeout callback's EVM call through `cachedCtx`, as the acknowledgement path already does.
+- Resolve the ABI method at the top of every stateful precompile's `Run`, before `RunNativeAction`
+  is entered. The native-action preamble takes a cache context, snapshots the multi-store and
+  commits the state DB cache - replaying the caller's entire dirty account and storage set - and
+  only then does `SetupABI` get around to resolving the selector and failing. `RequiredGas` returns
+  0 for an unresolvable selector, so none of that work was charged: a contract could dirty a large
+  storage set and then hand `bank`, `gov`, `staking`, `distribution`, `ics20`, `slashing`, `erc20`
+  or `werc20` four random bytes to have the whole set replayed for free, up to 20 times per
+  transaction. The check goes through the new `cmn.ResolveMethod`, which `SetupABI` now also uses,
+  so the fallback/receive special cases WERC20 depends on keep working and the revert is unchanged.
+- Make `ConvertCoinNativeERC20` atomic. It escrowed the sender's coins onto the erc20 module
+  account before calling the EVM, and `CallEVMWithData` caches only the EVM execution, so a VM
+  failure discarded the EVM cache while leaving the bank escrow committed. On the IBC error-ACK /
+  timeout refund path, which deliberately swallows a conversion error so a failed re-wrap cannot
+  undo the refund, that stranded the sender's coins on the module account permanently - contradicting
+  the `OnAcknowledgementPacket` / `OnTimeoutPacket` comments promising the user keeps the bank token.
+  The escrow, the EVM transfer and the burn now all run on a branched context and commit together.
+- Run the gov precompile's `getTallyResult` against a throwaway cache. The SDK's `TallyResult` query
+  delegates to `Keeper.Tally`, which deletes every vote it counts; that is safe on the SDK's own
+  query paths because a gRPC query runs against a context that is never committed, but precompiles
+  commit their state DB cache context unconditionally. A plain EVM call to `getTallyResult` on a
+  proposal in the voting period therefore deleted that proposal's votes permanently. The query's
+  store writes are now discarded, leaving the tally itself unchanged.
+- Verify the Ethereum sender in `Keeper.EthereumTx` before applying the transaction. The EVM ante
+  handler only runs over a tx's top-level messages, so an `MsgEthereumTx` nested inside a
+  dispatching module (`x/authz`, `x/group`, `x/gov`, a CosmWasm stargate/`Any` message, ICA) reached
+  the executor with its signature never checked, letting a victim-signed transaction be replayed as
+  the victim. `VerifySender` is now required before `ApplyTransaction`, so `From` is always the
+  ECDSA-recovered signer regardless of how the message arrived.
+- Clamp the fee market's cumulative gas wanted instead of failing the block. `EndBlock` converted
+  both the block's cumulative `gasWanted` and its `gasUsed` to `int64` and returned an error when
+  either did not fit. `gasWanted` is an unchecked uint64 sum of per-transaction declarations, and
+  under `max_gas: -1` nothing bounds a single declaration, so two transactions declaring `MaxInt64`
+  sum into `(MaxInt64, MaxUint64]` with neither being individually rejectable. An error out of
+  `EndBlock` surfaces through `FinalizeBlock` after the block has been decided, leaving CometBFT at
+  height H and the application at H-1. Both values are now clamped to `MaxInt64` and logged.
+  `AddTransientGasWanted` also saturates instead of wrapping, so the total can never silently roll
+  over to a small number.
+- Reject a `x/erc20` conversion whose token `transfer` emits an unexpected `Approval` event. Both
+  native-ERC20 convert directions escrow the token and mint (or burn) the paired coins, checking
+  only that the transfer succeeded and that the balance moved by the right amount. A registered
+  token that also grants a third party an allowance over the recipient satisfies both checks: the
+  escrow only drains later, when that allowance is spent, leaving the minted coins unbacked. The
+  `validateApprovalEventDoesNotExist` guard, already documented by both convert doc-comments but
+  never called, now runs on both paths.
+- Charge the parent Cosmos gas meter for failed EVM calls in `CallEVMWithData` and
+  `DerivedEVMCallWithData`. Both functions returned on `res.Failed()` before reaching
+  `ctx.GasMeter().ConsumeGas(res.GasUsed)`, so a reverting — or deliberately out-of-gas —
+  internal call performed real EVM work that cost the enclosing Cosmos transaction nothing
+  beyond the incidental KV-store gas. `DerivedEVMCallWithData` additionally clamps a
+  caller-supplied `gasLimit` to `config.DefaultGasCap`; on an out-of-gas halt `res.GasUsed`
+  equals the gas cap, so without the clamp the caller would pick how much gas the enclosing
+  transaction is forced to consume and could panic it with `OutOfGas`. `CallEVMWithData`
+  needs no clamp — its message gas limit is already the hardcoded `config.DefaultGasCap`.
+- Reject a `gasLimit` that does not fit in a `uint64` in `DerivedEVMCallWithData` instead of
+  truncating it. `big.Int.Uint64` is undefined for values that do not fit and in practice returns
+  the low 64 bits, and node-side validation bounds the universal-payload gas limit to `uint256` —
+  so a payload declaring `2^64 + 5` was silently handed **5** gas and failed with a misleading
+  out-of-gas error rather than a clear rejection. The call now returns `ErrInvalidGasLimit` naming
+  the problem; `math.MaxUint64` still fits and is clamped to `config.DefaultGasCap` as before.
 - [\#690](https://github.com/cosmos/evm/pull/690) Fix Ledger hardware wallet support for coin type 60.
 - [\#769](https://github.com/cosmos/evm/pull/769) Fix erc20 ibc middleware to not to validate sender address format.
 - [\#790](https://github.com/cosmos/evm/pull/790) fix panic in historical query due to missing EvmCoinInfo.

precompiles/bank/bank.go

@@ -107,6 +107,15 @@ func (p Precompile) RequiredGas(input []byte) uint64 {
 }
 
 func (p Precompile) Run(evm *vm.EVM, contract *vm.Contract, readonly bool) ([]byte, error) {
+	// Resolve the target method before RunNativeAction touches any state: the
+	// native-action preamble (cache context, multi-store snapshot, state DB
+	// commit) replays the caller's entire dirty set and is not paid for when the
+	// selector turns out to be unresolvable, because RequiredGas returns 0 for
+	// one. Resolving here keeps the revert identical while skipping that work.
+	if _, err := cmn.ResolveMethod(p.ABI, contract); err != nil {
+		return cmn.ReturnRevertError(evm, err)
+	}
+
 	return p.RunNativeAction(evm, contract, func(ctx sdk.Context) ([]byte, error) {
 		return p.Execute(ctx, contract, readonly)
 	})

precompiles/bank/bank_selector_test.go

@@ -0,0 +1,92 @@
+package bank_test
+
+import (
+	"math/big"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/vm"
+	"github.com/ethereum/go-ethereum/params"
+	"github.com/holiman/uint256"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cosmos/evm/precompiles/bank"
+	cmn "github.com/cosmos/evm/precompiles/common"
+)
+
+// runPrecompile drives bank.Precompile.Run with a nil StateDB and returns the
+// ABI-encoded revert reason.
+//
+// The early selector guard never looks at evm.StateDB, whereas the first thing
+// RunNativeAction does is type-assert on it. A revert reason carrying
+// cmn.ErrNotRunInEvm therefore proves the call reached the native-action
+// preamble, and any other reason proves it was rejected before it - which is
+// what lets these tests tell the two apart without standing up a full app.
+func runPrecompile(t *testing.T, input []byte, value *uint256.Int) string {
+	t.Helper()
+
+	evm := vm.NewEVM(vm.BlockContext{BlockNumber: big.NewInt(1)}, nil, params.TestChainConfig, vm.Config{})
+	contract := vm.NewContract(common.Address{}, common.Address{}, value, 100_000, nil)
+	contract.Input = input
+
+	bz, err := bank.NewPrecompile(nil, nil).Run(evm, contract, false)
+	require.ErrorIs(t, err, vm.ErrExecutionReverted, "precompile errors are always surfaced as a revert")
+
+	return string(bz)
+}
+
+// TestRunResolvesSelectorBeforeNativeAction covers F-2026-18786: calldata that
+// cannot be dispatched used to run the whole native-action preamble - cache
+// context, multi-store snapshot and a state DB commit that replays the caller's
+// entire dirty set - before SetupABI got around to failing on the selector, and
+// RequiredGas returns 0 for an unresolvable selector so none of it was charged.
+func TestRunResolvesSelectorBeforeNativeAction(t *testing.T) {
+	testCases := []struct {
+		name string
+		// input is the calldata handed to the precompile.
+		input []byte
+		// reachesNativeAction is true when the calldata is dispatchable and so
+		// must still be handed to RunNativeAction.
+		reachesNativeAction bool
+		// expReason, when set, must appear in the revert reason.
+		expReason string
+	}{
+		{
+			name:      "unknown selector is rejected before the native action",
+			input:     []byte{0xde, 0xad, 0xbe, 0xef},
+			expReason: "no method with id",
+		},
+		{
+			name:  "calldata shorter than a selector is rejected before the native action",
+			input: []byte{0x01, 0x02, 0x03},
+		},
+		{
+			name:  "empty calldata is rejected before the native action",
+			input: nil,
+		},
+		{
+			name:                "a valid selector is still dispatched",
+			input:               bank.ABI.Methods[bank.BalancesMethod].ID,
+			reachesNativeAction: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			reason := runPrecompile(t, tc.input, uint256.NewInt(0))
+
+			if tc.reachesNativeAction {
+				require.Contains(t, reason, cmn.ErrNotRunInEvm,
+					"a valid selector must still reach RunNativeAction")
+				return
+			}
+
+			require.NotContains(t, reason, cmn.ErrNotRunInEvm,
+				"undispatchable calldata must not reach the native-action preamble (F-2026-18786)")
+
+			if tc.expReason != "" {
+				require.Contains(t, reason, tc.expReason)
+			}
+		})
+	}
+}

precompiles/common/balance_handler_test.go

@@ -231,6 +231,51 @@ func TestAfterBalanceChange(t *testing.T) {
 	require.Equal(t, "3", stateDB.GetBalance(receiver).String())
 }
 
+// TestAfterBalanceChangeSpendMoreThanBalancePanics reproduces the F-2026-18201
+// attack path at the precompile boundary: the EVM state view only tracks
+// SPENDABLE balance, but a vesting account can delegate LOCKED coins through the
+// staking precompile. That emits a coin_spent event for more than the StateDB
+// holds. Before the underflow guard the subtraction wrapped to ~2^256, and
+// x/vm/keeper/statedb.go reconciled the bogus view back to bank by MINTING the
+// difference. It must now hard-fail instead.
+func TestAfterBalanceChangeSpendMoreThanBalancePanics(t *testing.T) {
+	setupBalanceHandlerTest(t)
+
+	storeKey := storetypes.NewKVStoreKey("test")
+	tKey := storetypes.NewTransientStoreKey("test_t")
+	ctx := sdktestutil.DefaultContext(storeKey, tKey)
+
+	stateDB := statedb.New(ctx, mocks.NewEVMKeeper(), statedb.NewEmptyTxConfig())
+
+	_, addrs, err := testutil.GeneratePrivKeyAddressPairs(1)
+	require.NoError(t, err)
+	spenderAcc := addrs[0]
+	spender := common.BytesToAddress(spenderAcc)
+
+	// Spendable balance as seen by the EVM.
+	stateDB.AddBalance(spender, uint256.NewInt(5), tracing.BalanceChangeUnspecified)
+
+	bankKeeper := cmnmocks.NewBankKeeper(t)
+	bankKeeper.Mock.On("BlockedAddr", mock.AnythingOfType("types.AccAddress")).Return(false)
+
+	bh := cmn.NewBalanceHandlerFactory(bankKeeper).NewBalanceHandler()
+	bh.BeforeBalanceChange(ctx)
+
+	// Delegating locked coins spends more than the spendable balance.
+	ctx.EventManager().EmitEvents(sdk.Events{
+		banktypes.NewCoinSpentEvent(spenderAcc, sdk.NewCoins(sdk.NewInt64Coin(evmtypes.GetEVMCoinDenom(), 10))),
+	})
+
+	require.PanicsWithValue(
+		t,
+		"state balance underflow for "+spender.Hex()+": have=5 sub=10",
+		func() { _ = bh.AfterBalanceChange(ctx, stateDB) },
+	)
+
+	// The balance must not have wrapped around to ~2^256.
+	require.Equal(t, "5", stateDB.GetBalance(spender).String())
+}
+
 func TestAfterBalanceChangeErrors(t *testing.T) {
 	setupBalanceHandlerTest(t)
 

precompiles/common/precompile.go

@@ -127,14 +127,19 @@ func (p Precompile) runNativeAction(evm *vm.EVM, contract *vm.Contract, action N
 	return bz, nil
 }
 
-// SetupABI runs the initial setup required to run a transaction or a query.
-// It returns the ABI method, initial gas and calling arguments.
-func SetupABI(
-	api abi.ABI,
-	contract *vm.Contract,
-	readOnly bool,
-	isTransaction func(name *abi.Method) bool,
-) (method *abi.Method, args []interface{}, err error) {
+// ResolveMethod resolves the ABI method targeted by the contract calldata.
+//
+// It is the selector-resolution half of SetupABI, exported so that a concrete
+// precompile can reject calldata it cannot dispatch at the very top of its Run,
+// *before* RunNativeAction sets up the native context, snapshots the multi-store
+// and commits the state DB cache. That preamble replays the caller's whole dirty
+// set and is not charged for an unresolvable selector, since RequiredGas also
+// returns 0 for one.
+//
+// SetupABI delegates to this function so the early check and the in-action check
+// can never disagree - notably on the fallback/receive special cases, which the
+// werc20 precompile relies on for empty, short and unknown calldata.
+func ResolveMethod(api abi.ABI, contract *vm.Contract) (method *abi.Method, err error) {
 	// NOTE: This is a special case where the calling transaction does not specify a function name.
 	// In this case we default to a `fallback` or `receive` function on the contract.
 
@@ -146,17 +151,29 @@ func SetupABI(
 	switch {
 	// Case 1: Calldata is empty
 	case isEmptyCallData:
-		method, err = emptyCallData(api, contract)
+		return emptyCallData(api, contract)
 
 	// Case 2: calldata is non-empty but less than 4 bytes needed for a method
 	case isShortCallData:
-		method, err = methodIDCallData(api)
+		return methodIDCallData(api)
 
 	// Case 3: calldata is non-empty and contains the minimum 4 bytes needed for a method
 	case isStandardCallData:
-		method, err = standardCallData(api, contract)
+		return standardCallData(api, contract)
 	}
 
+	return nil, vm.ErrExecutionReverted
+}
+
+// SetupABI runs the initial setup required to run a transaction or a query.
+// It returns the ABI method, initial gas and calling arguments.
+func SetupABI(
+	api abi.ABI,
+	contract *vm.Contract,
+	readOnly bool,
+	isTransaction func(name *abi.Method) bool,
+) (method *abi.Method, args []interface{}, err error) {
+	method, err = ResolveMethod(api, contract)
 	if err != nil {
 		return nil, nil, err
 	}

precompiles/distribution/distribution.go

@@ -95,6 +95,15 @@ func (p Precompile) RequiredGas(input []byte) uint64 {
 }
 
 func (p Precompile) Run(evm *vm.EVM, contract *vm.Contract, readonly bool) ([]byte, error) {
+	// Resolve the target method before RunNativeAction touches any state: the
+	// native-action preamble (cache context, multi-store snapshot, state DB
+	// commit) replays the caller's entire dirty set and is not paid for when the
+	// selector turns out to be unresolvable, because RequiredGas returns 0 for
+	// one. Resolving here keeps the revert identical while skipping that work.
+	if _, err := cmn.ResolveMethod(p.ABI, contract); err != nil {
+		return cmn.ReturnRevertError(evm, err)
+	}
+
 	return p.RunNativeAction(evm, contract, func(ctx sdk.Context) ([]byte, error) {
 		return p.Execute(ctx, evm.StateDB, contract, readonly)
 	})