Summary

If the ProxyFactory::distributeByOwner() is executed before the Proxy contract has been deployed, the transaction will be executed successfully, but the stuck tokens will not be transferred to a rescue requestor.

Vulnerability Details

The distributeByOwner() is used for recovering the rescue requestor's stuck tokens after a contest expires. The function will trigger the _distribute() to execute the contest's Proxy contract.

The transaction will not be reverted as expected if the Proxy has not been deployed before calling the distributeByOwner(). In other words, the transaction will be executed successfully, but the stuck tokens will not be transferred to the rescue requestor.

The root cause is that the _distribute() will make a low-level call to the Proxy without checking the existence of its contract.

• The distributeByOwner() triggers the _distribute(): https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/ProxyFactory.sol#L217

• The _distribute() makes a low-level call without checking the existence of the Proxy's contract: https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/ProxyFactory.sol#L250

• The event Distributed will be emitted regardless of whether the Proxy has a contract: https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/ProxyFactory.sol#L252

Impact

The rescue transaction will be executed successfully, but the stuck tokens will not be transferred to the rescue requestor. This incident can cause off-chain services to malfunction and confuse the owner (protocol admin) and the rescue requestor.

Tools Used

Manual Review

Recommendations

Verify that the target Proxy's address has a contract bytecode before executing the Proxy in the _distribute(), as below.