Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium

`organizer` losing tokens when owner set the `closeTime` incorrectly

savi0ur

Summary

Owner might set the contest with closeTime = block.timestamp, which will make a contest available for only current block and then in next block organizer have no choice but to distribute tokens at a cost of commission fee.

Vulnerability Details

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

function setContest(address organizer, bytes32 contestId, uint256 closeTime, address implementation)
public
onlyOwner
{
if (organizer == address(0) || implementation == address(0)) revert ProxyFactory__NoZeroAddress();
if (closeTime > block.timestamp + MAX_CONTEST_PERIOD || closeTime < block.timestamp) {
revert ProxyFactory__CloseTimeNotInRange();
}
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] != 0) revert ProxyFactory__ContestIsAlreadyRegistered();
saltToCloseTime[salt] = closeTime;
emit SetContest(organizer, contestId, closeTime, implementation);
}

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

if (closeTime > block.timestamp + MAX_CONTEST_PERIOD || closeTime < block.timestamp) {
revert ProxyFactory__CloseTimeNotInRange();
}

In setContest function, there is a check with closeTime. When closeTime = block.timestamp, we can see, this check fails and then it allows to set close time of a contest at
https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/ProxyFactory.sol#L114-L115

if (saltToCloseTime[salt] != 0) revert ProxyFactory__ContestIsAlreadyRegistered();
saltToCloseTime[salt] = closeTime;

When closeTime = block.timestamp, this contest set by owner is only valid for the current block. Once organizer see the contest is set by owner by using event emitted by setContest as emit SetContest(organizer, contestId, closeTime, implementation);, they will send tokens to address computed by getProxyAddress function.

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

function getProxyAddress(bytes32 salt, address implementation) public view returns (address proxy) {
bytes memory code = abi.encodePacked(type(Proxy).creationCode, uint256(uint160(implementation)));
bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), address(this), salt, keccak256(code)));
proxy = address(uint160(uint256(hash)));
}

Since contest is over due to closeTime < current block.timestamp, organizer have to distribute tokens to winners(supporters). But since contest got over immediately after its been set by owner, there is no winner, so organizer can pass their own address as winners and total percentage (BASIS_POINTS - COMMISSION_FEE) as percentages in distribute function to get their tokens back but at the cost of losing commission fee.

Note: owner is not allowed to reset closeTime once its set as if owner tries to reset this value, that call will get reverted due to check
https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/ProxyFactory.sol#L114

if (saltToCloseTime[salt] != 0) revert ProxyFactory__ContestIsAlreadyRegistered();

Impact

organizer losing their tokens when owner set the closeTime incorrectly.

Tools Used

Manual Review

Recommendations

We recommend to have some MIN_CONTEST_PERIOD and have a check on closeTime such that its atleast MIN_CONTEST_PERIOD further from current block.timestamp at the time of setContest

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.