Sparkn

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

Looping over arrays with arbitrary length can lead to DoS and freezed funds

cosine

Summary

The _distribute function in Distributor.sol loops over multiple arrays of arbitrary length and calls external contracts in a loop. There are a lot of different reasons why this can exceed the block gas limit and therefore lead to a temporary DoS / freezed funds. To fix this DoS and rescue the funds, an inconvenient process is required, which is highly prone to mistakes and could therefore lead to permanent loss of funds.

Vulnerability Details

When an organizer wants to send funds to the winners of a contest, a proxy must be deployed and the distribute function must be called:

function deployProxyAndDistribute(bytes32 contestId, address implementation, bytes calldata data)
public
returns (address)
{
bytes32 salt = _calculateSalt(msg.sender, contestId, implementation);
if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
// can set close time to current time and end it immediately if organizer wish
if (saltToCloseTime[salt] > block.timestamp) revert ProxyFactory__ContestIsNotClosed();
address proxy = _deployProxy(msg.sender, contestId, implementation);
_distribute(proxy, data);
return proxy;
}

The distribute function loops over multiple arrays of arbitrary length and calls external contracts:

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

There are multiple reasons why this function call could exceed the block gas limit, like for example the following ones, or a combination of them:

  • There are a lot of winners in the array

  • The token used is an ERC777 token and one or many of the winners are smart contract wallets with a receive function that wastes a lot of gas

  • The token uses a transfer function which wastes a lot of gas

If this call exceeds the block gas limit, it is not possible for anyone to distribute, the funds which are stucked inside the contract, to the supporters in the right way. The most efficient way to rescue and distribute the funds in the right way would be to distribute them all to the organizer, or owner instead and transfer them manually to the winner. But this approach would mean that the organizer, or owner, needs to transfer funds manually to hundreds or thousands of users. This is very inconvenient, the organizer must be trusted a lot and a mistake during that process could lead to permanent loss of funds.

Impact

Temporary DoS / freezed funds that can only be rescued with a very inconvenient and time-consuming way. This process can lead to a permanent loss of funds, if any mistakes are made accidentally or intentionally.

Tools Used

Manual Review, Foundry, VSCode

Recommendations

Use a deposit / withdraw mechanism to distribute the funds, or enable the possibility to call the distribute function multiple times and distribute less than 100% on these calls.

Support

FAQs

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