Sparkn

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

Contest "Organizer" can take all the funds

Summary

Given that the selection process for the distribution of prizes to winners occurs off-chain, there is nothing to prevent an Organizer of a contest from stealing contest funds by selecting winners from their accounts or accounts associated to them.

Vulnerability Details

Consider a scenario where a contest is created with cited parameters promising 10 DAI token prize pot split amongst 5 winners.
One or more Sponsors (not the Organizer) fund the contest with 10 DAI.

However, once the contest closeTime has been reached the Organizer simply triggers the deployProxyAndDistribute where the passed in data variable consists of a winners array with a single address controlled by the Organizer and a percentages array with a single value of 9500 basis points.

Thus the Organizer can steal the whole pot from the contest.

Impact

  1. Sponsors who fund the contest believing it to be fair will suffer financial loss

  2. Once it's known that an organizer can potentially manipulate the prize distribution to their advantage, trust in the entire platform or system will be compromised. This will very likely decrease user participation with it

  3. This is a strong case of centralization, if one user, the Organizer can exert such control it diminishes the protocol's appeal

Tools Used

Manual Review
Foundry

Recommendations

At a minimum there should be a check that the "Organizer" address is not one of the winner addresses in the data variable passed to the _distribute function in the proxy contract. See sample below where it's implemented in _distribute given that there is already a loop there:

function _distribute(
address token,
address[] memory winners,
uint256[] memory percentages,
bytes memory data,
address organizer
) internal {
// Ensure the organizer is not a winner
for (uint256 j = 0; j < winners.length; j++) {
if (winners[j] == organizer) {
revert Distributor__OrganizerCannotBeWinner();
}
}
// rest of function logic remains unchanged
}

Realistically however, the parameters of the contest should be stored on chain for transparency and there should be checks throughout the contract's functions to ensure the parameters are being met fairly and openly. E.g. if there are supposed to be 5 winners, that information is stored on chain and a check is done to confirm that 5 addresses are indeed passed in the data variable.

Support

FAQs

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

Give us feedback!