Caviar contest - BAHOZ's results

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L63

Vulnerability details

Impact

LPs are free to deposit liquidity in whatever ratio they choose, and they receive back LP tokens based on the min: https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L423

In systems like this, it is possible to provide liquidity in the wrong ratio, and that will cause the LP to lose funds effectively.

Other AMMs usually calculate an optimum ratio and deposit assets in that optimum ratio and refund the excess back to the LP. Check uniswap for ref: https://github.com/Uniswap/v2-periphery/blob/0335e8f7e1bd1e8d8329fd300aea2ef2f36dd19f/contracts/UniswapV2Router02.sol#L73

Recommended Mitigation Steps

Calculate optimum amounts to deposit in add(), and refund the excess amount

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L63

Vulnerability details

Impact

Creating the pool and making the first deposit are two separate actions. When the pool is first created, before the owner provides the first assets in a reasonable ratio, an attacker/griefer can attack the pool with an absurd ratio which will cause an imbalance between the assets in the pool.

Such an attack may cause two issues:

• if the depositor that comes after the attacker tries to deposit with a reasonable ratio and chooses a small slippage, it can not deposit because of the slippage protection.
• if the depositor that comes after the attacker tries to deposit with a reasonable ratio but does not choose a small slippage, the attacker can sandwich the depositor

Proof of Concept

Consider this scenario:

1. A new pool is created for a pair of an NFT collection named X and USDT.
2. Let's assume floor of NFT is around 1000 USDT, in that case it makes sense to start the pool with a nftAdd() with a single NFT and 1000e18 USDT. This will make the pool consist of 1000e18 USDT + 1e18 FracX(fractionised X NFT)
3. If an attacker wraps their own NFT to 1e18 FracX and adds liq before the owner's first deposit using a far different and absurd ratio, they can unbalance the pool.
4. Now, the liq deposits with reasonable ratios will be reverted because of the slip protection(if it is strict). The attacker can also front-run those deposits with more dust attacks for griefing purposes.
5. if the LPs deposit with a bigger slippage protection, the attacker can make a profit on them.

Tools Used

Recommended Mitigation Steps

Perform the first deposit to pool when it is first created, calling nftAdd() from the constructor.