validatePaymasterUserOp() call should not increase _withdrawableETH balance if signature is invalid

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-03-coinbase/blob/e0573369b865d47fed778de00a7b6df65ab1744e/src/MagicSpend/MagicSpend.sol#L137-L139

Vulnerability details

When the MagicSpend contract receives a validatePaymasterUserOp() call from the entryPoint contract, it will increase the balance a user is entitled to through the _withdrawableETH mapping. The issue is that this balance will be increased even though the signature is not valid.

Impact

Abuser will be able to add balance to their _withdrawableETH with invalid signatures enabling them to drain the MagicSpend account.

Proof of Concept

As we can see in the validatePaymasterUserOp function,

        _withdrawableETH[userOp.sender] += withdrawAmount - maxCost; 
        context = abi.encode(maxCost, userOp.sender);

withdrawAmount will be added to the balance of the sender. However if the signature is invalid this shouldn't be the case. This will allow an abuser to craft calls to increase their mapping balance with invalid signatures.

Tools Used

Manual review

Recommended Mitigation Steps

In order to mitigate this issue, if the signature happens to be invalid, the increase of the _withdrawableETH mapping should be ommitted

if(!sigFailed) {
       _withdrawableETH[userOp.sender] += withdrawAmount - maxCost; 
}

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

It would have been reverted in the simulation call by Entrypoint.

[wilsoncusack]

Abuser will be able to add balance to their _withdrawableETH with invalid signatures enabling them to drain the MagicSpend account.

This is not correct. This can only be called from the EntryPoint, which as @raymondfam, would revert on an invalid signature.

The logic here is important to gas estimation. We want to allow as many things as possible to "proceed as normal" with an invalid signature, so that we can get an accurate gas estimation.

[3docSec]

Echoing the lookout; the bundle reverts entirely if the paymaster validation returns anything else than a success.

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid