EIP4337Manager Self-Destruct Vulnerability

Author: taek lee

Auditing at Spearbit

---

Affected code: eth-infinitism/account-abstraction at 6aeb396

TL;DR

• develop branch’s EIP4337Manager can be destructed but does not risk user’s funds
• 0.4.0 release version gnosis based wallets are in risk of losing funds

Background

Gnosis Safe proxy supports “module” which means address that can call executeFromModule~() on safe. This is used to adding extensions for adding functionality in the safe contract.

Registration is done through safe.enableModule() function and EIP4337Fallback and EntryPoint is registered as module in this case.

Also, EIP4337Fallback is registered as fallback handler in Safe to enable functions not defined on gnosis singleton.

UserOp Flow

UserOp has to be “validated by sender” and “executed by sender”. So entrypoint first validates and executes the UserOp.

Validation

Validation of userOp is done through validateUserOp().

It is called by EntryPoint and Sender delegatecalls this data to Gnosis singleton contract. Gnosis singleton calls fallbackHandler with same msg.data since it does not have validateUserOp() in the implementation contract. As stated in Background, EIP4337Fallback is the fallbackHandler, Sender calls EIP4337Fallback’s validateUserOp().

validateUserOP() call flow #1 - validateUserOp()

And EIP4337Fallback calls back the Sender with execTransactionFromModuleReturnData() and forwards the msg.data. And Sender delegatecalls EIP4337Manager and executes validateUserOp().

validateUserOp() call flow #2 - execTransactionFromModuleReturnData()

Also, EntryPoint checks if Sender has enough deposits for gas usage before calling validateUserOp(). If not, it takes gas ETH during the validateUserOp().

Execution

If validation was successful, EntryPoint calls Sender with userOp.data for execution. In this case, it is using execTransactionFromModuleReturnData() according to test cases. And parses data from userOp.data and call target contract

Execution Process

Findings

My finding for the repository was simple and not very harmful at first glance.

Step 1 - EIP4337Manager can be selfdestructed

It is a classic issue from Parity Multisig wallet, “IF proxy’s implementation contract can be selfdestructed, proxy will lose logic and not function properly”.

Mostly, this is issue when implementation contract can be initialized directly. Attacker takes control of the implementation contract and selfdestructs the implementation contract.

Fortunately, Gnosis singleton contract has simple guard for this and does not allow calling setup() to implementation contract directly by setting threshold = 1 on constructor.

Safe.sol constructor guard

EIP4337Manager, which inherits Safe contract, has same blocker to prevent attacker from calling setup().

But, this contract does not need ownership for selfdestruct because setup4337Modules can be called by anyone.

By calling this function with malicious 4337Manager, anyone can easily become “module” for the contract and delegatecall to destructor contract.

PoC: self-destructing EIP4337Manager commit

Since Sender contract uses singleton contract as implementation contract, selfdestructing the EIP4337Manager looked harmless and i thought it will only cause DoS for wallet services since validateUserOp() won’t work.

Step 2 - signature-free userOps

Last night, i was trying to sleep and this issue hit my head again.

My PoC was about destructing the contract, not “validateUserOp() fails”. So… what if it does not fail? And, I don’t see any reason it should fail if EIP4337Manager got destroyed.

If we look at the diagram again, this part is where i assumed it will fail since EIP4337Manager is destructed.

But, suprising thing is, delegatecall/call/staticcall never fails on addresses without code.

Which means, EIP4337Fallback.validateUserOp() will not fail according to code. Instead, they will return “0” which means it did not failed.

So, every userOp can be executed through sender contract without any signature validation

PoC: signature-free UserOperation commit

But, it is now fixed with latest commit unintentionally.

This PR by derek last week eth-infinitism/account-abstraction#181 fixed the issue.

It was a simple PR that decodes the return data to uint256 to support more descriptive sigTimeRange to match the current spec. And for the exploit, since selfdestructed code does not return anything, it will try to decode zero length bytes and result in reverting the validation logic and stops userOp from getting executed.

Since this PR was added recently, if you are using EIP4337Manager code, there is high chance that this vulnerability will affect your service. So contact me asap