diff --git a/precompiles/common/balance_handler_test.go b/precompiles/common/balance_handler_test.go index 0e6c79930..a027c9d77 100644 --- a/precompiles/common/balance_handler_test.go +++ b/precompiles/common/balance_handler_test.go @@ -110,6 +110,14 @@ func TestParseAmount(t *testing.T) { }, expAmt: uint256.NewInt(5), }, + { + name: "unrelated denom is ignored", + maleate: func() sdk.Event { + coinStr := sdk.NewCoins(sdk.NewInt64Coin("foobar", 7)).String() + return sdk.NewEvent("bank", sdk.NewAttribute(sdk.AttributeKeyAmount, coinStr)) + }, + expAmt: uint256.NewInt(0), + }, { name: "missing amount", maleate: func() sdk.Event { @@ -186,6 +194,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) diff --git a/precompiles/common/utils.go b/precompiles/common/utils.go index 520dc5e09..4ca23a018 100644 --- a/precompiles/common/utils.go +++ b/precompiles/common/utils.go @@ -42,8 +42,18 @@ func ParseAmount(event sdk.Event) (*uint256.Int, error) { return nil, fmt.Errorf("failed to parse coins from %q: %w", amountAttr.Value, err) } - amountBigInt := amountCoins.AmountOf(evmtypes.GetEVMCoinDenom()).BigInt() - amount, err := utils.Uint256FromBigInt(evmtypes.ConvertAmountTo18DecimalsBigInt(amountBigInt)) + baseAmount := amountCoins.AmountOf(evmtypes.GetEVMCoinDenom()).BigInt() + extendedAmount := amountCoins.AmountOf(evmtypes.GetEVMCoinExtendedDenom()).BigInt() + + var amountBigInt *big.Int + if baseAmount.Sign() > 0 { + amountBigInt = evmtypes.ConvertAmountTo18DecimalsBigInt(baseAmount) + } else { + // The extended denom is already represented in 18 decimals. + amountBigInt = extendedAmount + } + + amount, err := utils.Uint256FromBigInt(amountBigInt) if err != nil { return nil, fmt.Errorf("failed to convert coin amount to Uint256: %w", err) } diff --git a/x/vm/statedb/state_object.go b/x/vm/statedb/state_object.go index db52e60c8..28e05c58b 100644 --- a/x/vm/statedb/state_object.go +++ b/x/vm/statedb/state_object.go @@ -2,6 +2,7 @@ package statedb import ( "bytes" + "fmt" "sort" "github.com/ethereum/go-ethereum/common" @@ -122,7 +123,16 @@ func (s *stateObject) SubBalance(amount *uint256.Int) uint256.Int { if amount.IsZero() { return *(s.Balance()) } - return s.SetBalance(new(uint256.Int).Sub(s.Balance(), amount)) + balance := s.Balance() + if balance.Lt(amount) { + panic(fmt.Sprintf( + "state balance underflow for %s: have=%s sub=%s", + s.address.Hex(), + balance.String(), + amount.String(), + )) + } + return s.SetBalance(new(uint256.Int).Sub(balance, amount)) } // SetBalance updates account balance. diff --git a/x/vm/statedb/statedb_test.go b/x/vm/statedb/statedb_test.go index 69bbddc2c..8cea4d214 100644 --- a/x/vm/statedb/statedb_test.go +++ b/x/vm/statedb/statedb_test.go @@ -239,8 +239,17 @@ func (suite *StateDBTestSuite) TestDBError() { } } +func (suite *StateDBTestSuite) TestSubBalanceUnderflowPanics() { + db := statedb.New(sdk.Context{}.WithEventManager(sdk.NewEventManager()), mocks.NewEVMKeeper(), emptyTxConfig) + db.AddBalance(address, uint256.NewInt(1), tracing.BalanceChangeUnspecified) + + expectedPanic := fmt.Sprintf("state balance underflow for %s: have=%s sub=%s", address.Hex(), "1", "2") + suite.Require().PanicsWithValue(expectedPanic, func() { + db.SubBalance(address, uint256.NewInt(2), tracing.BalanceChangeUnspecified) + }) +} + func (suite *StateDBTestSuite) TestBalance() { - // NOTE: no need to test overflow/underflow, that is guaranteed by evm implementation. testCases := []struct { name string malleate func(*statedb.StateDB)