The payable modifier on the multicall function does allow it to receive and hold ether, which can be locked in the contract forever

[c4-submissions]

Lines of code

https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L31

Vulnerability details

Impact

If Ether is sent to the multicall function, it will be stuck in the contract forever, since there is no way to get it out. This allows the owner to drain the contract's ether balance, recovering any accidentally sent funds.

Proof of Concept

The payable modifier on the multicall function allows it to receive and hold ether, which could potentially be locked in the contract forever without additional withdraw functionality. The payable modifier means this function can receive Ether when called. And there is no functionality to withdraw the Ether later. This means if Ether is sent to the multicall function, it will be stuck in the contract forever, since there is no way to get it out. A proof of concept would be:

1. A user calls multicall and accidentally sends 1 ether
2. That 1 ether is now stuck in the contract forever

So in summary: • The payable modifier allows the contract to receive/hold Ether • Without a withdraw function, Ether sent to the contract can be locked forever • This could be mitigated by adding a withdraw function to allow withdrawing Ether

Tools Used

Manual

Recommended Mitigation Steps

Additional withdraw functionality needs to be added

Assessed type

Other

[GalloDaSballo]

Sweep

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid