there is no slippage protection when depositing ERC20 or ERC721 tokens
Summary
users can deposit ETH, ERC20 or ERC721 token into the rounds. the issue is that the price of the ERC20 and ERC721 is calculated in contract with oracles and it may be different than the price when user signed their transaction. as result users may receive less entry point than what they wanted and their tokens deposited into rounds with unpredicted price that cause them loss.
Vulnerability Detail
function _deposit() allows users to deposit ERC20 and ERC721 into the rounds and it values those tokens based on the oracle price or the fixed price for the rounds. for ERC20 tokens the uniswap TWAP has been used as oracle price and for ERC721 token the Resavoir oracle has been used. the issue is that users can't specify price slippage when calling depisits() and as result if users transaction stay in the mempool for some time their tokens will be deposited with different price than when they signed their transaction.
This is like AMM slippage protection, in this code too, users tokens value is calculated and they receive entry points based on their tokens price so different price will result in different entry point and different winning chance which can result in loss for users.
the slippage will happen differently for ERC20 and ERC721 tokens:
for ERC20 tokens as the Uniswap TWAP has been used, if a round doesn't have a fixed price yet and user deposit his ERC20 token into that round then user would not have any control over the price that is going to be used to value his tokens. for example if there was 100ETH in that round and users ERC20 tokens worth 100ETH, user would assume he has 50% chance of wining if he deposit into that round. but by the time user tx get included in the blockchain user tokens value can be decreased in to 80ETH and now user won't have 50% winning chance.
for ERC721 tokens the Resavoir oracle has been used and user sends the oracle data with the tx. if a round doesn't have a fixed price yet and user wants to deposit his ERC721 token with a specific price and he would assume that price is going to be used to value his tokens, but if user tx stay in the mempool and another user deposit ERC721 into that round with different price then the price will be fixed for that round and user tx's price will be ignored and as result user tokens will be valued with different price which user have no control over it.
over all depositing ERC20 or ERC721 tokens into the rounds is like swapping tokens to ETH as users receive entry points based on ETH value of the deposited tokens and just like AMMs users should have slippage control mechanism to avoid deposits when the price changes dramatically by the time of transaction mining.
Impact
users would receive unfavorable price for their tokens and receive less entry point and less chance of winning. their tx can be front-run.
unforgiven
medium
there is no slippage protection when depositing ERC20 or ERC721 tokens
Summary
users can deposit ETH, ERC20 or ERC721 token into the rounds. the issue is that the price of the ERC20 and ERC721 is calculated in contract with oracles and it may be different than the price when user signed their transaction. as result users may receive less entry point than what they wanted and their tokens deposited into rounds with unpredicted price that cause them loss.
Vulnerability Detail
function
_deposit()allows users to deposit ERC20 and ERC721 into the rounds and it values those tokens based on the oracle price or the fixed price for the rounds. for ERC20 tokens the uniswap TWAP has been used as oracle price and for ERC721 token the Resavoir oracle has been used. the issue is that users can't specify price slippage when callingdepisits()and as result if users transaction stay in the mempool for some time their tokens will be deposited with different price than when they signed their transaction. This is like AMM slippage protection, in this code too, users tokens value is calculated and they receive entry points based on their tokens price so different price will result in different entry point and different winning chance which can result in loss for users.the slippage will happen differently for ERC20 and ERC721 tokens:
for ERC20 tokens as the Uniswap TWAP has been used, if a round doesn't have a fixed price yet and user deposit his ERC20 token into that round then user would not have any control over the price that is going to be used to value his tokens. for example if there was 100ETH in that round and users ERC20 tokens worth 100ETH, user would assume he has 50% chance of wining if he deposit into that round. but by the time user tx get included in the blockchain user tokens value can be decreased in to 80ETH and now user won't have 50% winning chance.
for ERC721 tokens the Resavoir oracle has been used and user sends the oracle data with the tx. if a round doesn't have a fixed price yet and user wants to deposit his ERC721 token with a specific price and he would assume that price is going to be used to value his tokens, but if user tx stay in the mempool and another user deposit ERC721 into that round with different price then the price will be fixed for that round and user tx's price will be ignored and as result user tokens will be valued with different price which user have no control over it.
over all depositing ERC20 or ERC721 tokens into the rounds is like swapping tokens to ETH as users receive entry points based on ETH value of the deposited tokens and just like AMMs users should have slippage control mechanism to avoid deposits when the price changes dramatically by the time of transaction mining.
Impact
users would receive unfavorable price for their tokens and receive less entry point and less chance of winning. their tx can be front-run.
Code Snippet
https://github.com/sherlock-audit/2024-01-looksrare/blob/7d76b96a58a6aee38f23bb38b8a5daa3bdc03f7c/contracts-yolo/contracts/YoloV2.sol#L1029-L1179
Tool used
Manual Review
Recommendation
users should be able to specify slippage protection for the price of their tokens.
Duplicate of #50