sherlock-audit / 2024-01-looksrare-judging

3 stars 0 forks source link

mert_eren - with depositETHIntoMultipleRounds can deposit 0 eth to rounds and it can effect the winner of round #154

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

mert_eren

high

with depositETHIntoMultipleRounds can deposit 0 eth to rounds and it can effect the winner of round

Summary

depositETHIntoMultipleRounds can be used for bet current and future rounds at the same time. It just check msg.value should be bigger than 0, sum of the contents of amounts array and msg.value should be equal and amounts.length should be bigger than 0 but not look at any number in amounts array is bigger than 0 or not. This will give a chance to user to deposit 0 amount to any wanted round including current round and this will cause to duplicate numbers in entryIndex array and will break the functionality of findUpperBounds() function when fullfillRandomWords() function determine the winner.

Vulnerability Detail

There is a test i provide which shows 0 deposit can be happen in currentRound with depositETHIntoMultipleRounds().

    function test_YYYY() public{
        uint256 looksAmount = 1_000 ether;
        // 4th user deposits 1,000 LOOKS
        deal(LOOKS, user4, 2*looksAmount);

        IYoloV2.DepositCalldata[] memory depositsCalldata = new IYoloV2.DepositCalldata[](1);
        depositsCalldata[0].tokenType = IYoloV2.YoloV2__TokenType.ERC20;
        depositsCalldata[0].tokenAddress = LOOKS;
        uint256[] memory amounts = new uint256[](1);
        amounts[0] = looksAmount;
        depositsCalldata[0].tokenIdsOrAmounts = amounts;

        _grantApprovalsToTransferManager(user4);

        vm.startPrank(user4);
        IERC20(LOOKS).approve(address(transferManager), looksAmount);
        yolo.deposit(1, depositsCalldata);
        vm.stopPrank();
        vm.deal(user2, 1 ether);
        vm.startPrank(user2);
        uint[] memory amounts2=new uint[](2);
        amounts2[0]=0;
        amounts2[1]=0.5 ether;
        vm.stopPrank();
        (,,,,,,,,,IYoloV2.Deposit[] memory deposits)=yolo.getRound(1);
        uint first=deposits[0].currentEntryIndex;
        uint second=deposits[1].currentEntryIndex;
        assertEq(first,second);
        }

And from test we can also see that first deposit entry and second deposit entry has same currentEntryIndex. This is problematic and it can be showed with a given scenerio: Let assume that after round end the current entryIndex look like this: [2,4,5,5,8,9,11]. The second 5 occured due to 0 deposit can be possible. And winner number is 5. At this point normally the first 5 should be winner of roound but due to findUpperBounds() function assume that there is no duplicate and clearly function broke with that array, the second 5 will be the winner of this round which is the unwanted scenerio for protocol.

Impact

0 deposit can be done with depositETHIntoMultipleRounds and it would cause unwanted scenerious when determine the winner.

Code Snippet

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

Tool used

Manual Review

Recommendation

Duplicate of #18