Sparkn

Summary

Low-level calls call/delegatecall/staticcall return true even if the account called is non-existent (per EVM design). Solidity documentation warns: "The low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed.”

Reference: https://docs.soliditylang.org/en/v0.8.18/control-structures.html#error-handling-assert-require-revert-and-exceptions

Vulnerability Details

In ProxyFactory.sol, the function _distribute() is as follows:

According to the Solidity docs, "The low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed".

As a result, it is possible that this call will fail, but _distribute() will not notice anything went wrong. In particular, it is possible that the proxy hasn't been deployed yet, but _distribute() will not revert and continue to emit the event of prizes distribution. If proxy is indeed a non-existent contract, it would be better for _distribute() to revert until the organizer or owner manually deploy the proxy based on the salt.

Impact

When the owner call distributeByOwner() to rescue the fund, sponsor(s) mistakenly sent funds may be locked up temporarily as distributed event was emitted successfully. Causing some convenience for the sponsor(s) as they have to contact the protocol which is time-consuming.

POC

Paste this function into ProxyFactoryTest.t.sol:

https://gist.github.com/sonny2k/33357d0aba7d94677bd2b5250229cce0

To run this, type forge test --mt testSucceedsNonExistenceProxyDistributeByOwner -vvvvv

As you can see the distributed event is emitted with the proxy address of 0x1, which is a non-existence proxy address in the console logs.

Tools Used

Manual Analysis

Recommendations

Check for contract existence on low-level calls, so that failures are not missed.