Sparkn

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

The same contestId can be assigned for different contests

serialcoder

Summary

The same contestId param can be assigned for different contests, causing off-chain services to malfunction.

Vulnerability Details

The contestId param is supposed to be globally unique, randomly generated off-chain (confirmed by the developer). However, when an owner executes the ProxyFactory::setContest() to register a contest, the function does not check the double spending of the given contestId param.

In other words, it is possible to use the same contestId for different contests under different organizers. Moreover, the same contestId can even be used for different contests under the same organizer if the Implementation contract is upgraded.

@> 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);
}
​
...
​
@> function _calculateSalt(address organizer, bytes32 contestId, address implementation)
internal
pure
returns (bytes32)
{
@> return keccak256(abi.encode(organizer, contestId, implementation));
}

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

  • The _calculateSalt(): https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/ProxyFactory.sol#L260-L266

Impact

The double spending of the contestId param can cause off-chain services to malfunction.

Tools Used

Manual Review

Recommendations

Use the mapping variable to track and verify all spent contestId(s) in the setContest().

Support

FAQs

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