sherlock-audit / 2024-01-looksrare-judging

3 stars 0 forks source link

pontifex - The number of deposits for future rounds may exceed `MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND` #137

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

pontifex

medium

The number of deposits for future rounds may exceed MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND

Summary

Due to the lack of appropriate checks in function depositETHIntoMultipleRounds, the number of deposits in future rounds may exceed MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND. This violates the expected behavior of the function and will possibly lead to an error due to lack of gas in the fulfillRandomWords function.

Vulnerability Detail

The depositETHIntoMultipleRounds function only check if roundDepositCount == MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND. In case the roundDepositCount of the next round is already equals MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND the new deposit will be added when the round will became current. Once the number of deposits exceeds bbb, they can be added without restrictions.

    function _shouldDrawWinner(
        uint256 numberOfParticipants,
        uint256 maximumNumberOfParticipants,
        uint256 roundDepositCount
    ) private pure returns (bool shouldDraw) {
        shouldDraw =
            numberOfParticipants == maximumNumberOfParticipants ||
            (numberOfParticipants > 1 && roundDepositCount == MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND);
    }

Impact

The code does not work as expected. In case of error at the fulfillRandomWords, the protocol will lose the fee for the failed round.

Code Snippet

https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L353-L361 https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L1718

Tool used

Manual Review

Recommendation

Consider checking that current round does not exceed the MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND with the same check as at the _deposit function.

Duplicate of #78