[M] deployProxyAndDistribute doesnt check if the contest has expired before distributing prizes

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/ProxyFactory.sol#L127

https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/ProxyFactory.sol#L205

Summary

The deployProxyAndDistribute function is designed to deploy a proxy contract and distribute prizes immediately after the contest is closed. It checks whether the contest is closed (saltToCloseTime[salt] > block.timestamp) and then proceeds to distribute the prizes.

Vulnerability Details

The distributeByOwner function is designed to allow the owner to distribute prizes on behalf of the organizer, but only after the contest is expired. It checks whether the contest is expired (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) and then proceeds to distribute the prizes.

Impact

In the scenario described, if the owner calls distributeByOwner after the contest is expired and the deployProxyAndDistribute function is called around the same time by another party (e.g., a frontrunner), there could indeed be a possibility of duplicate distributions.

This could happen because the deployProxyAndDistribute function doesn't check if the contest is expired; it only checks if it's closed. If someone exploits the timing, they could execute the deployProxyAndDistribute function before the owner's distributeByOwner transaction, resulting in duplicate distributions.

Tools Used

Manual Review

Recommendations

You could add a boolean flag to check if prizes have been distributed or not.

Alternatively, you can add a modifier to the function to check if prizes have been distributed or not.

i.e

// Define the modifier
modifier prizesDistributed(bytes32 salt) {
    require(hasDistributed[salt] == false, "Prizes already distributed for this contest");
    _;
    hasDistributed[salt] = true; // Set the flag after successful distribution
}

And then insert the modifier into the functions' signature.

Cyfrin / 2023-08-sparkn