dany.armstrong90 - Distribution of entries to users is wrong.

[sherlock-admin2]

dany.armstrong90

high

Distribution of entries to users is wrong.

Summary

When a user deposits several tokenIds of ERC721, it does not distribute entries corresponding to total value. This problem occurs in case of ERC20.

Vulnerability Detail

YoloV2.sol#_deposit() function where a user deposits is as follows.

    function _deposit(uint256 roundId, DepositCalldata[] calldata deposits) private {
        Round storage round = rounds[roundId];
        _validateRoundIsOpen(round);

        ...

        if (deposits.length != 0) {
            ITransferManager.BatchTransferItem[] memory batchTransferItems = new ITransferManager.BatchTransferItem[](
                deposits.length
            );

            for (uint256 i; i < deposits.length; ++i) {
                DepositCalldata calldata singleDeposit = deposits[i];
                address tokenAddress = singleDeposit.tokenAddress;
                if (isCurrencyAllowed[tokenAddress] != 1) {
                    revert InvalidCollection();
                }
                uint256 price = prices[tokenAddress][roundId];
                if (singleDeposit.tokenType == YoloV2__TokenType.ERC721) {
                    if (price == 0) {
                        price = _getReservoirPrice(singleDeposit);
                        prices[tokenAddress][roundId] = price;
                    }

1102                uint256 entriesCount = price / round.valuePerEntry;
                    if (entriesCount == 0) {
                        revert InvalidValue();
                    }

                    uint256[] memory amounts = new uint256[](singleDeposit.tokenIdsOrAmounts.length);
                    for (uint256 j; j < singleDeposit.tokenIdsOrAmounts.length; ++j) {
                        totalEntriesCount += entriesCount;

                        ...

                        amounts[j] = 1;
                    }

                    ...
                } else if (singleDeposit.tokenType == YoloV2__TokenType.ERC20) {
                    if (price == 0) {
                        price = erc20Oracle.getTWAP(tokenAddress, uint32(TWAP_DURATION));
                        prices[tokenAddress][roundId] = price;
                    }

                    uint256[] memory amounts = singleDeposit.tokenIdsOrAmounts;
                    if (amounts.length != 1) {
                        revert InvalidLength();
                    }

                    uint256 amount = amounts[0];

                    uint256 entriesCount = ((price * amount) / (10 ** IERC20(tokenAddress).decimals())) /
                        round.valuePerEntry;
                    if (entriesCount == 0) {
                        revert InvalidValue();
                    }

                    ...
                } else {
                    revert InvalidTokenType();
                }
            }

            transferManager.transferBatchItemsAcrossCollections(batchTransferItems, msg.sender, address(this));
        }

        ...

        emit Deposited(msg.sender, roundId, totalEntriesCount);
    }

As we can see above, the entriesCount is calculated separately about individual tokenIds. For example, we say that the price of token is 150, round.valuePerEntry == 100 and a user deposits two tokenIds. Then total value is 300 but it distributes only 2 entries. This problem occurs in case of ERC20.

This is unfair to users. In fact, it is little possible that the price of ERC721 token is multiple of round.valuePerEntry. So it means that a user who deposits ERC721 always loses funds.

Impact

When a user deposits ERC721 token, he does not recieve correct count of entries corresponding to total value. This problem exists in case of ERC20.

Code Snippet

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

Tool used

Manual Review

Recommendation

YoloV2.sol#_deposit function has to be modified as follows.

    function _deposit(uint256 roundId, DepositCalldata[] calldata deposits) private {
        Round storage round = rounds[roundId];
        _validateRoundIsOpen(round);

        ...

        if (deposits.length != 0) {
            ITransferManager.BatchTransferItem[] memory batchTransferItems = new ITransferManager.BatchTransferItem[](
                deposits.length
            );

+           uint256 extraValue;
            for (uint256 i; i < deposits.length; ++i) {
                DepositCalldata calldata singleDeposit = deposits[i];
                address tokenAddress = singleDeposit.tokenAddress;
                if (isCurrencyAllowed[tokenAddress] != 1) {
                    revert InvalidCollection();
                }
                uint256 price = prices[tokenAddress][roundId];
                if (singleDeposit.tokenType == YoloV2__TokenType.ERC721) {
                    if (price == 0) {
                        price = _getReservoirPrice(singleDeposit);
                        prices[tokenAddress][roundId] = price;
                    }

-                   uint256 entriesCount = price / round.valuePerEntry;
-                   if (entriesCount == 0) {
-                       revert InvalidValue();
-                   }

+                   uint256 unitCount = price / round.valuePerEntry;
+                   if(unitCount == 0){
+                       revert InvalidValue();
+                   }
+                   uint256 entryExtraPrice = price % round.valuePerEntry;

                    uint256[] memory amounts = new uint256[](singleDeposit.tokenIdsOrAmounts.length);
                    for (uint256 j; j < singleDeposit.tokenIdsOrAmounts.length; ++j) {
+                       uint256 entriesCount = unitCount;
+                       extraValue += entryExtraPrice;
+                       entriesCount += extraValue / round.valuePerEntry;
+                       extraValue = extraValue % round.valuePerEntry;

                        totalEntriesCount += entriesCount;

                        ...

                        amounts[j] = 1;
                    }

                    ...
                } else if (singleDeposit.tokenType == YoloV2__TokenType.ERC20) {
                    if (price == 0) {
                        price = erc20Oracle.getTWAP(tokenAddress, uint32(TWAP_DURATION));
                        prices[tokenAddress][roundId] = price;
                    }

                    uint256[] memory amounts = singleDeposit.tokenIdsOrAmounts;
                    if (amounts.length != 1) {
                        revert InvalidLength();
                    }

                    uint256 amount = amounts[0];

-                   uint256 entriesCount = ((price * amount) / (10 ** IERC20(tokenAddress).decimals())) /
-                       round.valuePerEntry;

+                   extraValue += (price * amount) / (10 ** IERC20(tokenAddress).decimals();
+                   uint256 entriesCount = extraValue / round.valuePerEntry;
+                   extraValue = extraValue % round.valuePerEntry;

                    if (entriesCount == 0) {
                        revert InvalidValue();
                    }

                    ...
                } else {
                    revert InvalidTokenType();
                }
            }

            transferManager.transferBatchItemsAcrossCollections(batchTransferItems, msg.sender, address(this));
        }

        ...

        emit Deposited(msg.sender, roundId, totalEntriesCount);
    }

Duplicate of #11