gnosis/EIP4337Manager can be destructed and lead to DoS of wallets

[leekt]

PoC : https://github.com/leekt/account-abstraction/commit/c21e13fc5f0b3bcaa9b87ab726916b6ec9c4bb8e

EIP4337Manager implementation contract can be destructed with delegatecall although it's setup() function cannot be called directly since it is blocked with simple threshold = 1, anyone can call setup4337Modules() without any authorization to enable delegatecall from malicious actor

Since EIP4337Manager is not a direct implementation contract for proxy, funds are not at risk.

But, this will lead to DoS for ERC4337 features which will potentially damage the business logic of wallet provider

remediation: add simple blocker for calling function directly to implementation contract for setup4337Modules and replaceEIP4337Manager

address immutable addressThis;
constructor() {
    addressThis = address(this);
}

modifier notThis {
    require(address(this) != addressThis);
    _;
}

[drortirosh]

The check is indeed required. Can you add a PR for that?

(it would be best of Eip4337Manager was not a full "Safe". however, since it uses a lot of safe members (threshold, nonce, modules), it is tricky to implement it without being a Safe, and without a lot of copy-and-paste from the Safe code..)

[leekt]

Sure :) i'll start working on the fixes