This repository was archived by the owner on Apr 4, 2024. It is now read-only.

Description
This code here https://github.com/tharsis/ethermint/blob/b8ae5984c54f79d76cfc95518f77b4226a05b940/x/evm/keeper/utils.go#L83-L94
test that the keeper has sufficient balance to pay for the cost of a transaction.
However, notice that it does a big.Int comparison https://github.com/tharsis/ethermint/blob/b8ae5984c54f79d76cfc95518f77b4226a05b940/x/evm/keeper/utils.go#L86
Essentially if the account balance ever gets negative, there could be a scenario in which I could craft TxData to have a "negative" cost where it is being used as a big.Int which show it as negative but perhaps much later on, that cost could get cast to a uint* which will then wrap around to a positive value.
Proposal
For safety and consistency, let's ensure that we reject negative costs with an explicit check
if cost.Sign() < 0 {
// Report the error here about the cost being negative.
return stacktrace.Propagate(
sdkerrors.Wrapf(
sdkerrors.ErrInsufficientFunds, // Or whatever error it is.
"tx cost (%s%s) is negative and invalid", txData.Cost(), denom,
),
...
)
}
// Same for checking if the balance is >= 0
balanceBigInt := balance.Amount.BigInt()
if balanceBigInt.Sign() < 0 {
// Report that the balance has dipped less than 0.
return stacktrace.Propagate(
....
}
}
// Now finally do the prior comparisons
if balance.Amount.BigInt().Cmp(cost) < 0 {
return ...
}