Potential DoS when the list of winner is large

[codehawks-bot]

Potential DoS when the list of winner is large

Severity

Low Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/Distributor.sol#L145-L151

Summary

When list of winner is relatively long, the _distribute function may suffer a DoS, as the result the reward cannot be distributed.

Vulnerability Details

We can measure the gas cost with the following test(based on the given testbase):

    function createVerboseData(uint256 num) public returns (bytes memory data) {
        address[] memory tokens_ = new address[](num);
        for (uint i = 0; i < num; i++) {
            tokens_[i] = jpycv2Address;
        }
        address[] memory winners = new address[](num);
        for (uint i = 0; i < num; i++) {
            string memory x = vm.toString(i);
            winners[i] = makeAddr(x);
        }        
        uint256[] memory percentages_ = new uint256[](num);
        uint256 t = 9500/num; // try no decimal
        for (uint i = 0; i < num; i++) {
            percentages_[i] = t; //95*100
        }        

        data = abi.encodeWithSelector(Distributor.distribute.selector, jpycv2Address, winners, percentages_, "");
    }

    function testSucceedsWhenConditionsAreMet2() public setUpContestForJasonAndSentJpycv2Token(organizer) {
        // before
        assertEq(MockERC20(jpycv2Address).balanceOf(user1), 0 ether);
        assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 0 ether);

        bytes32 randomId_ = keccak256(abi.encode("Jason", "001"));
        bytes memory data = createVerboseData(500);
        // console.log(proxyFactory.saltToCloseTime(keccak256(abi.encode(organizer, randomId_, address(distributor)))));
        // console.log(proxyFactory.whitelistedTokens(jpycv2Address)); // true
        // console.log(distributor._isWhiteListed(jpycv2Address)); // true

        vm.warp(9 days); // 9 days later
        vm.startPrank(organizer);
        proxyFactory.deployProxyAndDistribute(randomId_, address(distributor), data);
        vm.stopPrank();

        // after
        // assertEq(MockERC20(jpycv2Address).balanceOf(user1), 9500 ether);
        // assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 500 ether);
    }

Take 100 and 500 as example, the gas cost for the deployProxyAndDistribute will be:

• 500 winners

  | Function Name                              | min             | avg     | median  | max     | # calls |
  | deployProxyAndDistribute                   | 2926448         | 2926448 | 2926448 | 2926448 | 1       |

• 500 winners

  | Function Name                              | min             | avg      | median   | max      | # calls |
  | deployProxyAndDistribute                   | 14348116        | 14348116 | 14348116 | 14348116 | 1       |

We can see when 500 winners' in the list, the transaction will be close to the hardcap of the Ethereum(Noted: the value will be vary cross different chain. And during the test we assume the account will not have the token before the distribution, which try to simulate the cold storage. Also, since the token implementation may varied, the result can also be different)

https://ethereum.org/en/developers/docs/gas/#block-size

Impact

Since the use case can be varied with sparkn, the number of winner cannot be determined during the auditing. As a result, if the list of winner is pretty long, the DoS may happen.

Tools Used

Manual

Recommendations

It is better to distribute the reward in different batch in case of the long winner list, or the contract can allow user claim the reward by themselves to avoid expensive operation in the loop.