Potential DOS or out of gas exception due to unbounded loop

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/Distributor.sol#L92-L99

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/Distributor.sol#L116-L156

Summary

The contract lacks validation for the array lengths of winners[] and percentages[] in the distribute(). The percentages provided in the percentages[] and winners[] are used in a loop within the _distribute(). If these arrays are large, the loop can consume an excessive amount of gas, leading to transaction failure and even a denial-of-service attack.

Vulnerability Details

When rewards are distributed, the system loops through the list of winners and their percentages to send them the rewards they are entitled to. Because this loop is unbounded and the number of receivers can grow, the amount of gas consumed is also unbounded. Additionally, if one of the winner is a contract, code that significantly increases the gas cost of the distribute will execute.

Impact

This vulnerability can lead to an out-of-gas and DOS issue during the execution of the _distribute(). If the contract is used for distributing tokens to a significant number of winners, an attacker could intentionally provide large arrays, causing legitimate transactions to fail or even disrupting the contract's functionality.

Tools Used

Manual Review

Recommendations

Examine the execution cost of the function to determine the safe bounds of the loop and, if possible, consider splitting the distribution operation into multiple calls.
Consider redesigning the reward distribution mechanism to avoid unbounded loops and prevent denials of service.

Cyfrin / 2023-08-sparkn