Sparkn

Summary

The Owner can use the distributeByOwner function to distribute funds from a proxy for a contest which has not yet expired, which is not intended.

Vulnerability Details

The function distributeByOwner uses the salt of a contest (calculated based on the organizer, contestId and implementation) to check that the contest has expired, and if the contest has not expired, the function will revert with error ProxyFactory__ContestIsNotExpired()

However, the address of the proxy that is actually called in _distribute(proxy, data); is supplied as an input parameter by the Owner and there is no check that this proxy address corresponds to the salt.

Suppose there are two contests, A and B. Contest A has expired while B has not expired. The Owner can call distributeByOwner and provide the organizer, contestId and implementation of contest A, but provide the proxy address of contest B. The salt (for contest A) would pass the expiration check, but the _distribute function would be called using the address of the proxy for contest B, even though it hasn't expired.

Impact

While the Owner is a trusted party, there are clearly meant to be restrictions on the Owner's ability to distribute tokens in a proxy, such that the Owner is only able to distribute tokens from a proxy after the contest's expiration. This is evidenced by the contest details (which states "the owner can distribute the tokens stuck in the proxy after the proxy's expiration"), the natspec (which says "Owner can rescue funds if token is stuck after the deployment and contest is over for a while") and the presence of the following logic in the function: if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired();. This intended restriction is bypassed.

Tools Used

Manual Review

Recommendations

Instead of having the proxy address as a parameter input by the Owner, the proxy address should be calculated from the salt and implementation using the getProxyAddress function. This will ensure that the proxy address which is called will correspond to the salt calculated from the organizer, contestId and implementation provided by the Owner, which would prevent the expiration check from being bypassed.