code-423n4 / 2024-03-coinbase-findings

1 stars 0 forks source link

`SELFBALANCE` opcode cannot be used during validation #193

Closed c4-bot-9 closed 8 months ago

c4-bot-9 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-coinbase/blob/main/src/MagicSpend/MagicSpend.sol#L133-L135

Vulnerability details

Impact

The contract balance cannot be queried in the validation code due to storage access restrictions of ERC-4337.

Proof of Concept

The Paymaster uses the SELFBALANCE opcode during the validation phase:

if (address(this).balance < withdrawAmount) {
    revert InsufficientBalance(withdrawAmount, address(this).balance);
}

https://github.com/code-423n4/2024-03-coinbase/blob/main/src/MagicSpend/MagicSpend.sol#L133-L135

This is one of the Opcodes that it's blocked during validation:

    BALANCE (0x31)
    ORIGIN (0x32)
    GASPRICE (0x3A)
    BLOCKHASH (0x40)
    COINBASE (0x41)
    TIMESTAMP (0x42)
    NUMBER (0x43)
    PREVRANDAO/DIFFICULTY (0x44)
    GASLIMIT (0x45)
@>  SELFBALANCE (0x47)
    BASEFEE (0x48)
    GAS (0x5A)
    CREATE (0xF0)
    INVALID (0xFE)
    SELFDESTRUCT (0xFF)

https://docs.stackup.sh/docs/opcode-rules#op-011-blocked-opcodes

Additional info about this issue can be found in the TokenPaymaster template, even if it's a different model (it uses ERC20 as payment):

    /// @title Sample ERC-20 Token Paymaster for ERC-4337
    /// This Paymaster covers gas fees in exchange for ERC20 tokens charged using allowance pre-issued by ERC-4337 accounts.
    /// The contract refunds excess tokens if the actual gas cost is lower than the initially provided amount.
@>  /// The token price cannot be queried in the validation code due to storage access restrictions of ERC-4337.
    /// The price is cached inside the contract and is updated in the 'postOp' stage if the change is >10%.
    /// It is theoretically possible the token has depreciated so much since the last 'postOp' the refund becomes negative.
    /// The contract reverts the inner user transaction in that case but keeps the charge.
    /// The contract also allows honest clients to prepay tokens at a higher price to avoid getting reverted.
    /// It also allows updating price configuration and withdrawing tokens by the contract owner.
    /// The contract uses an Oracle to fetch the latest token prices.
    /// @dev Inherits from BasePaymaster.

https://github.com/eth-infinitism/account-abstraction/blob/develop/contracts/samples/TokenPaymaster.sol#L15-L25

Tools Used

Manual review

Recommended Mitigation Steps

It's difficult to provide a solution to this as it impacts the entire design, but blocked opcodes should be avoided during the validation phase.

Assessed type

Context

raymondfam commented 8 months ago

Known issue per contest readme: validatePaymasterUserOp checks address.balance, which currently violates ERC-7562 rules, however there is PR to change this.

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as primary issue

c4-judge commented 8 months ago

3docSec marked the issue as unsatisfactory: Out of scope