Sparkn

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

Multiple Unbounded Loops

Summary

There were multiple unbounded loops used in the _distribute function

Vulnerability Details

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;
}
}

In the distribute function in the Distributor.sol, there are two loops being implemented, one calculates the totalPercentage and the other send tokens to users, considering the amount of gas already spent before this function is called from deployAndDistribute function and similar functions. A large enough list of winner and their percentage could easily cause the deployAndDistribute function and a similar call to go over the gasLimit. Also it could revert halfway done with the distribution and cause a number of users to lose out.

Impact

Incomplete distribution, data inconsistency

Tools Used

Manual Review

Recommendations

There are two recommendations by me;

  • The first is to put a limit on the list of winners that could get the token rewards.

  • The second is to add a cache to save the response of the transfer for each winner and a function to redistribute. If a transfer is unsuccessful, either by malicious contract or gas limit, it will revert but the contract will have been created and a number of users will have received their funds but with a number of winners left. The redistribute will attempt to only distribute and will skip those already successful

Support

FAQs

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