Skinny Pickle Barracuda
High
The process of depositing ERC20 or ETH from L1 to L2 begins with the depositTransaction function in the OptimismPortal2 contract. This contract can be called directly by users or through the L1CrossDomainMessenger to mint or transfer ERC20 or ETH on L2. The value is determined by msg.value and forwarded to the TransactionDeposited event, which is then listened to by nodes and processed in blocks. Although L2StandardBridge checks token balances before withdrawal operations (
withdraw->_initiateWithdrawal->_initiateBridgeERC20>`OptimismMintableERC20(_localToken).burn(_from, _amount);). When executing _burn, it will first check whether the current account has a sufficient token balance to be burned. But! The lack of underflow checks in the SubBalance operation presents a potential issue in the op-chain-ops, which should be addressed.
The MemoryStateDB implementation and state.goallows an account's balance to become negative when the SubBalance function subtracts more than the available balance. This is due to the absence of checks to prevent underflow, violating Ethereum’s requirement that balances must always be non-negative.
The SubBalance function does not verify if the account has sufficient funds before subtracting. This lack of an underflow check allows the balance to become negative if the subtraction exceeds the account's current balance. This behavior could lead to inconsistencies or exploitation. https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/op-chain-ops/state/memory_db.go#L82-L95 https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/op-chain-ops/state/state.go#L51-L65
Vulnerable code snippet:
func (db *MemoryStateDB) SubBalance(addr common.Address, amount *uint256.Int) {
db.rw.Lock()
defer db.rw.Unlock()
account, ok := db.genesis.Alloc[addr]
if !ok {
panic(fmt.Sprintf("%s not in state", addr))
}
if account.Balance.Sign() == 0 {
return
}
account.Balance = new(big.Int).Sub(account.Balance, amount.ToBig()) // 缺少下溢检查
db.genesis.Alloc[addr] = account
}-
Logical Inconsistency In Ethereum, account balances should always be non-negative. If the underlying system doesn't enforce this, negative balances might occur when upper-level checks are insufficient, leading to inconsistent behavior with the Ethereum protocol. For example, an account with a negative balance might still be allowed to make transactions, which contradicts expected behavior.
-
Attack Vectors
Without underflow checks, attackers could exploit this to manipulate balances: Malicious Contracts: Smart contracts might not expect negative balances, allowing for exploits under certain conditions.
- Subsequent Logic Errors
A negative balance can make subsequent balance-related calculations unreliable. For example, comparisons or aggregations involving negative balances can lead to further logic errors, potentially causing system-wide issues.
- Inconsistent with Economic Model
Balances reflect on-chain assets, and allowing negative balances contradicts blockchain’s economic model, as no account should have negative assets. Ethereum enforces this rule, making negative balances invalid from an economic perspective.
Manual Review
To prevent this, add an underflow check in the SubBalance function to ensure balances remain non-negative. Here's a suggested modification:
func (db *MemoryStateDB) SubBalance(addr common.Address, amount *uint256.Int) {
db.rw.Lock()
defer db.rw.Unlock()
account, ok := db.genesis.Alloc[addr]
if !ok {
panic(fmt.Sprintf("%s not in state", addr))
}
// Underflow check to prevent negative balance
if account.Balance.Cmp(amount.ToBig()) < 0 {
panic(fmt.Sprintf("insufficient balance for address %s", addr))
}
account.Balance = new(big.Int).Sub(account.Balance, amount.ToBig())
db.genesis.Alloc[addr] = account
}