app/ante/evm: CanTransfer should reject any message.Value() with a negative amount

[odeke-em]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

While auditing this code I noticed this pattern

evmos/app/ante/evm/07_can_transfer.go

Lines 49 to 56 in ad1e289

	if msg.Value().Sign() > 0 && !evm.Context.CanTransfer(stateDB, msg.From(), msg.Value()) {
	return errorsmod.Wrapf(
	errortypes.ErrInsufficientFunds,
	"failed to transfer %s from address %s using the EVM block context transfer function",
	msg.Value(),
	msg.From(),
	)
	}

which presumes that the cost of a message will always be either 0 or positive. That's all gucci to think about, but what happens if for example a message with an actual negative value is passed in? That check will not reject it and later on it'll be propagated across the stack because it passed the "CanTransfer" check and later on when calculating gas costs and deducting balances, we could even end up with increasing the account balance :-( (this could possibly even be a security vulnerability)

Remedy

Please firstly check that the amount is negative and if so, reject it with a direct error, then also please add a regression test to the code

+if msg.Value() == nil || msg.Value().Sign() == -1 {
+     return errors.New("message.Value must be positive")
+}

/cc @fedekunze

Evmos Version

main

How to reproduce?

Please read the bug report above

[linear]

ENG-2553 app/ante/evm: CanTransfer should reject any message.Value() with a negative amount