unnecessary rounding error when depositing ERC721 may cause loss for users and contract won't support low value ERC721 tokens
Summary
users can deposit multiple ERC721 tokens into round by calling deposit() function. the issue is that code calculates the value of the NFT tokens separately and this would cause multiple rounding error. users would receive way less entry points for their NFTs.
Vulnerability Detail
in deposit() function, when users deposits ERC721 tokens, contracts calculates the entry points with uint256 entriesCount = price / round.valuePerEntry and then for each deposited NFT it allocate entriesCount for the user. this is like performing division before multiplication. for example:
suppose valuePerEntry is 1 ETH and USER1 wants to deposit NFT1 into the a round and floor price of the NFT1 is 1.8ETH.
now USER1 calls deposit() and deposits 10 token of NFT1 into the round ID1.
code would calculate the valuePerEntry = 1.8 / 1 = 1 and it would set 10 * 1 = 1 entry point for the USER1.
now USER1 deposited 10 NFT token worth of 18 ETH while receiving only 10 entry point worth of 10 ETH. so the USER1 winning chance would not differ from what value they deposited. a USER who deposit 10ETH would have the same chance of winning.
as result users who deposits multiple NFT tokens would receive less entry points than what they deserve.
this could be avoided if code registered all those IDs in one deposit and set the entriesCount = price * ID_count / valuePerEntry
Impact
users who deposit multiple ERC721 tokens receive less entry point than what their tokens worth and they would have bad risk/reward ratio.
unforgiven
medium
unnecessary rounding error when depositing ERC721 may cause loss for users and contract won't support low value ERC721 tokens
Summary
users can deposit multiple ERC721 tokens into round by calling
deposit()function. the issue is that code calculates the value of the NFT tokens separately and this would cause multiple rounding error. users would receive way less entry points for their NFTs.Vulnerability Detail
in
deposit()function, when users deposits ERC721 tokens, contracts calculates the entry points withuint256 entriesCount = price / round.valuePerEntryand then for each deposited NFT it allocateentriesCountfor the user. this is like performing division before multiplication. for example:valuePerEntryis 1 ETH and USER1 wants to deposit NFT1 into the a round and floor price of the NFT1 is 1.8ETH.deposit()and deposits 10 token of NFT1 into the round ID1.valuePerEntry = 1.8 / 1 = 1and it would set10 * 1 = 1entry point for the USER1.this could be avoided if code registered all those IDs in one deposit and set the
entriesCount = price * ID_count / valuePerEntryImpact
users who deposit multiple ERC721 tokens receive less entry point than what their tokens worth and they would have bad risk/reward ratio.
Code Snippet
https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L1096-L1119
Tool used
Manual Review
Recommendation
code should calculate entry point for all the NFTs and assign it to the user instead of calculating one NFT entry points.
Duplicate of #11