Uploaded files for judging

sherlock-audit · Jan 24, 2024 · c0f3ca6 · c0f3ca6
1 parent 7f60502
commit c0f3ca6
Show file tree

Hide file tree

Showing 164 changed files with 12,494 additions and 10 deletions.
diff --git a/.gitignore b/.gitignore
diff --git a/001.md b/001.md
@@ -0,0 +1,21 @@
+Clever Carrot Sheep
+
+high
+
+# Integer Overflow in `_startRound` function
+
+krkba
+## Summary
+The function uses the `unchecked` keyword to increment `roundId`.
+## Vulnerability Detail
+`roundId` is calculated by incrementing  `_roundsCount` by 1, and the `unchecked` is used to disable the automatic overflow and underflow checks.
+If `_roundsCount` is large(maximum uint256) and incrementing it by 1 will result an overflow, the value of `roundId` will be around to a very small number or zero.
+## Impact
+Integer overflow can occur other vulnerabilities or it can used by the attacker to minipulate the contract. 
+## Code Snippet
+https://github.com/sherlock-audit/2024-01-looksrare/blob/main/contracts-yolo/contracts/YoloV2.sol#L949-L952
+## Tool used
+
+Manual Review
+
+## Recommendation
diff --git a/001/002.md b/001/002.md
@@ -0,0 +1,179 @@
+Acrobatic Flint Lobster
+
+high
+
+# Griefing Vulnerability: The `MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND` on future rounds can be wasted on zero-value deposits.
+
+## Summary
+
+Each round imposes a [`MAXIMUM_NUMBER_OF_DEPOSITS_PER_ROUND`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L81), which represents a scarce upper boundary on the number of individual deposits which can be made before a round is considered closed and the winnings are drawn.
+
+The expectation is that at a minimum, a single deposit must at least be the [`valuePerEntry`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L197).
+
+However, this requirement can be bypassed by an attacker, who can flood future [`YoloV2`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol) rounds with empty deposits - reducing liveliness of playability, and in the worst case, introducing significant value-extraction opportunities through frontrunning.
+
+## Vulnerability Detail
+
+When accepting multi-round deposits via [`depositETHIntoMultipleRounds(uint256[])`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L312C14-L312C70), we only verify the sum-total of  [`amounts`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L312C14-L312C70) is equal to the `msg.value`, but do not validate that each individual array element satisfies the [`roundValuePerEntry`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L338C33-L338C51):
+
+```solidity
+function depositETHIntoMultipleRounds(uint256[] calldata amounts) external payable nonReentrant whenNotPaused {
+    uint256 numberOfRounds = amounts.length;
+    if (msg.value == 0 || numberOfRounds == 0) {
+        revert ZeroDeposits();
+    }
+
+    // ...
+
+    uint256 expectedValue;
+    uint256[] memory entriesCounts = new uint256[](numberOfRounds);
+
+    for (uint256 i; i < numberOfRounds; ++i) {
+    
+        // ...
+
+        uint256 depositAmount = amounts[i];
+        
+@>      if (depositAmount % roundValuePerEntry != 0) { /// @audit accepts_zero
+            revert InvalidValue();
+        }
+        uint256 entriesCount = _depositETH(round, roundId, roundValuePerEntry, depositAmount);
+        expectedValue += depositAmount;
+
+        entriesCounts[i] = entriesCount;
+    }
+
+    if (expectedValue != msg.value) {
+        revert InvalidValue();
+    }
+
+    // ...
+    
+}
+```
+
+This allows an attacker to create an array of [`amounts`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L312C14-L312C70) where a single array element is equal to the `msg.value`, [which must be non-zero](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L314), and all other array elements are `0`.
+
+In the following example, we demonstrate how an attacker may flood `99` future rounds of [`YoloV2`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol) with empty deposits:
+
+```solidity
+function test_sherlock_consumeDeposits() public {
+    uint256[] memory amounts = new uint256[](1);
+    amounts[0] = 10 ether;
+
+    vm.deal(user1, 10 ether);
+    vm.prank(user1);
+    yolo.depositETHIntoMultipleRounds{value: 10 ether}(amounts);
+
+    vm.deal(user2, 100 ether);
+    amounts = new uint256[](100);
+    amounts[0] = 1 ether;
+
+    // Grief the next 100 rounds.
+    for (uint256 i = 0; i < 99; i += 1) {
+        vm.prank(user2);
+        yolo.depositETHIntoMultipleRounds{value: 1 ether}(amounts);
+    }
+
+    for (uint256 i = 0; i < 99; i += 1) {
+        (,,,,,,,,, IYoloV2.Deposit[] memory deposits) = yolo.getRound(i + 2);
+        assertEq(deposits.length, 99);
+    }
+}
+```
+
+The implication here is that the capital efficiency, liveliness and fairness of `99` future rounds have been manipulated significantly.
+
+Additionally, it must be emphasised that there is little restrictive financial incentive against a wealthy attacker from capitalising upon this vulnerability, since securing illiquid deposits on future rounds comes only at the expense of securing a high win probability for the current round, as the non-zero `msg.value` is accumulated there, effectively through "normal" play.
+
+## Impact
+
+I've come to evaluate this as high severity, since the illiquid deposit amount can be abused for profit, as it reduces the game to a closed two-party interaction between attacker and victim.
+
+If we consider the case where an attacker has flooded the future N rounds with `98` deposits, they can monitor the mempool in anticipation of an unsuspecting user submitting an attractive deposit, and backrun that transaction in an effort to procure it.
+
+When an unsuspecting user makes a valuable [`deposit(uint256,DepositCalldata[])`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L305C14-L305C75), the attacker has the freedom to immediately backrun their transaction with a deposit value greatly in excess of the honest user's deposit, in an effort to secure a probabilistically-high (near certainty) of winning through immediate termination of the round - this may seem like conventional YOLO, however in lieu of the zero deposits issue, such an attack would be significantly more expensive.
+
+There are several other medium-severity implications of this vulnerability:
+1. Attackers can circmvent the [`roundValuePerEntry`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L338C33-L338C51) invariant for both current and future rounds by distributing their `msg.value` evenly over the [`amounts`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L312C14-L312C70).
+2. Congestion to users, as the next N rounds can be forced to terminate immediately on each deposit.
+3. Denial of Service if the empty deposits are programmatically configured to originate from multiple addresses, satisfying [`numberOfParticipants > 1`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L1718C14-L1718C38) for round completion, forcing rounds to be closed manually via [`cancel(uint256)`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L429C14-L429C44), at expense of the [`owner`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L430).
+4. A zero-value deposit will steal the win from a rightful owner for a fulfilled randomness value which rewards the final entry count share, as both of these will modulo to 0, and the malicious entry will possess higher priority in the array. This means when making an ether deposit, there is little financial incentive against callers appending hundreds of zero-value future deposits, or by sandwiching new deposits with zero-value entrants to capture a higher-priority `entryCount`.
+5. Invalidation of unique element constraint for [`findUpperBound(uint256[], uint256)`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/libraries/Arrays.sol#L20C14-L20C70).
+
+## Code Snippet
+
+```solidity
+/**
+ * @inheritdoc IYoloV2
+ */
+function depositETHIntoMultipleRounds(uint256[] calldata amounts) external payable nonReentrant whenNotPaused {
+    uint256 numberOfRounds = amounts.length;
+    if (msg.value == 0 || numberOfRounds == 0) {
+        revert ZeroDeposits();
+    }
+
+    uint256 startingRoundId = roundsCount;
+    Round storage startingRound = rounds[startingRoundId];
+    _validateRoundIsOpen(startingRound);
+
+    _setCutoffTimeIfNotSet(startingRound);
+
+    uint256 expectedValue;
+    uint256[] memory entriesCounts = new uint256[](numberOfRounds);
+
+    for (uint256 i; i < numberOfRounds; ++i) {
+        uint256 roundId = _unsafeAdd(startingRoundId, i);
+        Round storage round = rounds[roundId];
+        uint256 roundValuePerEntry = round.valuePerEntry;
+        if (roundValuePerEntry == 0) {
+            (, , roundValuePerEntry) = _writeDataToRound({roundId: roundId, roundValue: 0});
+        }
+
+        _incrementUserDepositCount(roundId, round);
+
+        uint256 depositAmount = amounts[i];
+        if (depositAmount % roundValuePerEntry != 0) {
+            revert InvalidValue();
+        }
+        uint256 entriesCount = _depositETH(round, roundId, roundValuePerEntry, depositAmount);
+        expectedValue += depositAmount;
+
+        entriesCounts[i] = entriesCount;
+    }
+
+    if (expectedValue != msg.value) {
+        revert InvalidValue();
+    }
+
+    emit MultipleRoundsDeposited(msg.sender, startingRoundId, amounts, entriesCounts);
+
+    if (
+        _shouldDrawWinner(
+            startingRound.numberOfParticipants,
+            startingRound.maximumNumberOfParticipants,
+            startingRound.deposits.length
+        )
+    ) {
+        _drawWinner(startingRound, startingRoundId);
+    }
+}
+```
+
+## Tool used
+
+Vim, Foundry
+
+## Recommendation
+
+### [YoloV2.sol#depositETHIntoMultipleRounds](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L312)
+
+Ensure that when creating a new deposit when batching into multiple rounds that the [`roundValuePerEntry`](https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L338C33-L338C51) is met:
+
+```diff
+uint256 depositAmount = amounts[i];
+- if (depositAmount % roundValuePerEntry != 0) {
++ if (depositAmount % roundValuePerEntry != 0 && depositAmount > 0) {
+    revert InvalidValue();
+}
+```
diff --git a/001/003.md b/001/003.md
@@ -0,0 +1,129 @@
+Sparkly Cloth Moose
+
+high
+
+# Users could deposit 0 ETH into the round
+
+## Summary
+
+Users could deposit 0 ETH into the round via `depositETHIntoMultipleRounds` function, leading to the increase of `round.deposits` and `round.numberOfParticipants`. This situation may result in the round ending prematurely and impact the overall fairness of the game.
+
+## Vulnerability Detail
+
+When depositing ETH into multiple rounds, there is a validation check solely for the total ETH amount, overlooking individual checks for each amount in the `amounts` array. This oversight enables any user to deposit 0 ETH into the rounds, as long as the total amount of ETH is not zero.
+
+An attacker could exploit this to prematurely conclude the round and gain a profit from it.
+
+```solidity
+function depositETHIntoMultipleRounds(uint256[] calldata amounts) external payable nonReentrant whenNotPaused {
+        uint256 numberOfRounds = amounts.length;
+        //@audit only check the total msg.value is not zero
+        if (msg.value == 0 || numberOfRounds == 0) {
+            revert ZeroDeposits();
+        }
+
+        uint256 startingRoundId = roundsCount;
+        Round storage startingRound = rounds[startingRoundId];
+        _validateRoundIsOpen(startingRound);
+
+        _setCutoffTimeIfNotSet(startingRound);
+
+        uint256 expectedValue;
+        uint256[] memory entriesCounts = new uint256[](numberOfRounds);
+
+        for (uint256 i; i < numberOfRounds; ++i) {
+            uint256 roundId = _unsafeAdd(startingRoundId, i);
+            Round storage round = rounds[roundId];
+            uint256 roundValuePerEntry = round.valuePerEntry;
+            if (roundValuePerEntry == 0) {
+                (, , roundValuePerEntry) = _writeDataToRound({roundId: roundId, roundValue: 0});
+            }
+
+            _incrementUserDepositCount(roundId, round);
+//@audit amounts[i] could be zero here
+            uint256 depositAmount = amounts[i];
+            if (depositAmount % roundValuePerEntry != 0) {
+                revert InvalidValue();
+            }
+            uint256 entriesCount = _depositETH(round, roundId, roundValuePerEntry, depositAmount);
+            expectedValue += depositAmount;
+
+            entriesCounts[i] = entriesCount;
+        }
+
+        if (expectedValue != msg.value) {
+            revert InvalidValue();
+        }
+
+        emit MultipleRoundsDeposited(msg.sender, startingRoundId, amounts, entriesCounts);
+
+        if (
+            _shouldDrawWinner(
+                startingRound.numberOfParticipants,
+                startingRound.maximumNumberOfParticipants,
+                startingRound.deposits.length
+            )
+        ) {
+            _drawWinner(startingRound, startingRoundId);
+        }
+    }
+```
+
+Add the test code to `contracts-yolo/test/foundry/Yolo.depositETHIntoMultipleRounds.t.sol` and run it with `forge test --match-test test_depositETHIntoMultipleRounds_With_Zero`.
+
+```diff
+diff --git a/contracts-yolo/test/foundry/Yolo.depositETHIntoMultipleRounds.t.sol b/contracts-yolo/test/foundry/Yolo.depositETHIntoMultipleRounds.t.sol
+index 338d2c7..a290a38 100644
+--- a/contracts-yolo/test/foundry/Yolo.depositETHIntoMultipleRounds.t.sol
++++ b/contracts-yolo/test/foundry/Yolo.depositETHIntoMultipleRounds.t.sol
+@@ -494,6 +494,17 @@ contract Yolo_DepositETHIntoMultipleRounds_Test is TestHelpers {
+         yolo.depositETHIntoMultipleRounds{value: 10.009 ether}(amounts);
+     }
+
++    function test_depositETHIntoMultipleRounds_With_Zero()
++        public
++        asPrankedUser(user1)
++    {
++        vm.deal(user1, 10 ether);
++        uint256[] memory amounts = _amounts();
++        amounts[0] = 0 ether;
++        amounts[1] = 3 ether;
++        yolo.depositETHIntoMultipleRounds{value: 10 ether}(amounts);
++    }
++
+     function test_depositETHIntoMultipleRounds_RevertIf_OnePlayerCannotFillUpTheWholeRound_FirstRound() public {
+         vm.deal(user1, 1_000 ether);
+         for (uint256 i; i < 99; ++i) {
+```
+
+## Impact
+
+Users have the ability to deposit 0 ETH into the round, causing an increase in both `round.deposits` and `round.numberOfParticipants`. This scenario may contribute to the round ending prematurely and potentially affect the overall fairness of the game.
+
+## Code Snippet
+
+https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L312-L362
+
+## Tool used
+
+Forge 
+
+## Recommendation
+
+It's recommended to check the value of every amount in the `amounts` array.
+
+```diff
+diff --git a/contracts-yolo/contracts/YoloV2.sol b/contracts-yolo/contracts/YoloV2.sol
+index b8afe96..0591dc1 100644
+--- a/contracts-yolo/contracts/YoloV2.sol
++++ b/contracts-yolo/contracts/YoloV2.sol
+@@ -335,7 +335,7 @@ contract YoloV2 is
+             _incrementUserDepositCount(roundId, round);
+
+             uint256 depositAmount = amounts[i];
+-            if (depositAmount % roundValuePerEntry != 0) {
++            if (depositAmount ==0 || depositAmount % roundValuePerEntry != 0) {
+                 revert InvalidValue();
+             }
+             uint256 entriesCount = _depositETH(round, roundId, roundValuePerEntry, depositAmount);
+```