Potentially unsafe use of forceSafeTransferETH

[c4-bot-9]

Lines of code

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

Vulnerability details

Impact

When a user use an intermediate contract to pass along a call without the ability for this contract to receive their refund, the eth will be stuck in the intermediate contract.

Proof of Concept

postOp will send the all remaining funds to the user account using forceSafeTransferETH:

        if (withdrawable > 0) {
            SafeTransferLib.forceSafeTransferETH(account, withdrawable, SafeTransferLib.GAS_STIPEND_NO_STORAGE_WRITES);
        }

This function works by attempting to send the value to the address and, if it fails, creating a temporary contract with the balance and self destructing it to force the ETH to the address:

    function forceSafeTransferETH(address to, uint256 amount, uint256 gasStipend) internal {
        /// @solidity memory-safe-assembly
        assembly {
            if lt(selfbalance(), amount) {
                mstore(0x00, 0xb12d13eb) // `ETHTransferFailed()`.
                revert(0x1c, 0x04)
            }
            if iszero(call(gasStipend, to, amount, codesize(), 0x00, codesize(), 0x00)) {
                mstore(0x00, to) // Store the address in scratch space.
                mstore8(0x0b, 0x73) // Opcode `PUSH20`.
                mstore8(0x20, 0xff) // Opcode `SELFDESTRUCT`.
                if iszero(create(amount, 0x0b, 0x16)) { revert(codesize(), codesize()) } // For gas estimation.
            }
        }
    }

The issue is that, in the case of a mint, it could be harmful for the refunded ETH to be forced to the contract.

For example, let's consider a situation where a user uses the Multicaller contract at 0x000000000088228fCF7b8af41Faf3955bD0B3A41. This contract would become the msg.sender for their mint and would receive the gas refund. Since it doesn't have a receive() function, it would ordinarily revert, but in this case would have the ETH forced into the contract balance. It would then become a race between the user and MEV bots to extract the ETH.

This is one example, but this same issue exists in any situation where a user might use an intermediate contract to pass along a call without the ability for this contract to receive their refund.

Tools Used

Manual review

Recommended Mitigation Steps

Consider using safeTransferETH instead of forceSafeTransferETH, to allow the user to determine a different call path (or send the correct msg.value to avoid there being a refund).

Assessed type

ETH-Transfer

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[raymondfam]

Typically, the sender is the entryPoint while account is SCW. This should generally not pose an issue but will let the sponsor review.

[c4-sponsor]

wilsoncusack marked the issue as disagree with severity

[wilsoncusack]

Account is UserOp.sender, which must be able to respond to validation queries and we expect to be a smart account. It would have to be an intentional abuse of the system to have sender be some address where you lose control of ETH.

[c4-sponsor]

wilsoncusack (sponsor) acknowledged

[3docSec]

Requiring refunds to the multicall address where they are up for grabs for anyone is a user mistake.

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid