Summary

The comment of the _distribute() states that:

 * The winners and percentages array are supposed not to be so long, so the loop can stay unbounded

But there are no code assurances that this is not the case, see Vulnerability Details

Vulnerability Detail

Take a look at Distributor.sol#L85-L156

Click to see code reference

/**

* @notice Distribute token to winners according to the percentages

* @dev Only factory contract can call this function

* @param token The token address to distribute

* @param winners The addresses array of winners

* @param percentages The percentages array of winners

*/

function distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)

external

{

if (msg.sender != FACTORY_ADDRESS) {

revert Distributor__OnlyFactoryAddressIsAllowed();

}

_distribute(token, winners, percentages, data);

}

​

/**

* @notice An internal function to distribute JPYC to winners

* @dev Main logic of distribution is implemented here. The length of winners and percentages must be the same

* The token address must be one of the whitelisted tokens

* The winners and percentages array are supposed not to be so long, so the loop can stay unbounded

* The total percentage must be correct. It must be (100 - COMMITION_FEE).

* Finally send the remained token(fee) to STADIUM_ADDRESS with no dust in the contract

* @param token The token address

* @param winners The addresses of winners

* @param percentages The percentages of winners

* @param data The data to be logged. It is supposed to be used for showing the realation bbetween winners and proposals.

*/

​

function _distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)

internal

{

// token address input check

if (token == address(0)) revert Distributor__NoZeroAddress();

if (!_isWhiteListed(token)) {

revert Distributor__InvalidTokenAddress();

}

// winners and percentages input check

if (winners.length == 0 || winners.length != percentages.length) revert Distributor__MismatchedArrays();

uint256 percentagesLength = percentages.length;

uint256 totalPercentage;

for (uint256 i; i < percentagesLength;) {

totalPercentage += percentages[i];

unchecked {

++i;

}

}

// check if totalPercentage is correct

if (totalPercentage != (10000 - COMMISSION_FEE)) {

revert Distributor__MismatchedPercentages();

}

IERC20 erc20 = IERC20(token);

uint256 totalAmount = erc20.balanceOf(address(this));

​

// if there is no token to distribute, then revert

if (totalAmount == 0) revert Distributor__NoTokenToDistribute();

​

uint256 winnersLength = winners.length; // cache length

for (uint256 i; i < winnersLength;) {

uint256 amount = totalAmount * percentages[i] / BASIS_POINTS;

erc20.safeTransfer(winners[i], amount);

unchecked {

++i;

}

}

​

// send commission fee as well as all the remaining tokens to STADIUM_ADDRESS to avoid dust remaining

_commissionTransfer(erc20);

emit Distributed(token, winners, percentages, data);

}

​

Where as at the early stages of the CodeFox's SparkN project one can say this would not be an issue in the future there could be contest with a lot of winners, this is because there is presently no limit on the amount of people that can participate in contests, and in a case either the amount of winners get unfairly reduced cause the loop could get too large Additionally do note that from [the SparkN's contest onboarding video](https://www.youtube.com/watch?v=_VqXB1t9Evo) the teams have specified that this project could even work with trying to solve issues that are government related, with MarkWu, specifically hinting a question in the lines of _What can the government do to tackle citizens moving from rural areas to the cities_ This seems to be a pretty interesting question but at the same time means that a lot of people could provide interesting ideas and even be "winner worthy" also depending on how big SparkN gets the amount of people ready to participate would massively increase, where as this is not 100% really related, take Code4rena as an example, at the early stages of the protocol, you can even have a contest where ~ 20 people competed, but as of last month Code4rena has over 5000 registered Wardens from across the world, and we can both agree that SparkN would even take a shorter time frame to reach this level of adoption since they are not really in a niche, say smart contract auditing in the case of code and can have any type of contest

Impact

Impact I believe is a high, since it would only take time before this happens, where as this seems impossible as at the early stages of the protocol, once it gets big enough there would complete inability to distribute rewards to winners whenever a contest has a lot of winners

Tool used

Manual Audit

Recommendation

Refactor the _distribute() function to take this into account, i.e create the possibility of calling it by batches, alternatively, one could set a limit for the amount of participant a contest can have to ensure this never happens.... but the latter option easily cripples adoption