Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L211 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L301

Vulnerability details

Impact

The buy and sell functions have no option to supply a minimum amount to receive or maximum price to pay. Transactions can easily be frontrun to change the prices, exposing the user to risk of very unfavourable trades.

Proof of Concept

Bob wants to buy an NFT from a PrivatePool, paying witn an ERC20 token. He calls the buy() function with the desired tokenIDs.

a MEV BOTS sees this in mempool and does a sandwich attack. Bot buys some NFTs from the PrivatePool, driving up the price it Exexutes Bob's transaction. Bob pays more then expected Sells back the NFTs from step 1 with a profit.

Even worse, since the creation of pools is permissionless, anyone can create a pool. An evil owner can create a pool and setup a frontrunning bot. When some user wants to buy or sell, he can frontrun that transaction and call setVirtualReserves() to change the prices at will. This can result in users paying very hogh prices in their buy, or receiving almost 0 when selling an NFT.

Tools Used

Manual review

Recommended Mitigation Steps

Add a maxInputAmount parameter the buy() function and a minReceivedAmount parameter to the sell() function to protect users from big price movements by frontrunners or other price changes.

code-423n4 / 2023-04-caviar-findings