Caviar contest - bytehat's results

Lines of code

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

Vulnerability details

Impact

When calculating how many LP tokens to mint to the user for their deposit, the code uses the min of their ratio of the total baseToken and their ratio of the total fractionalToken. However, the code doesn't refund the user for the excess of the token that didn't contribute to the minimum of these ratios (in other words, if these ratios are not the same for both tokens, then the user will be transferring in more tokens than they needed to for one of the two). This flaw allows a frontrunner to manipulate the token reserve amounts to lead to a user receiving much less value than they wanted, even if they receive their full minLpTokenAmount.

In UniswapV2, the router handles this logic correctly by only transferring the exact amount of tokens that are needed. Sending any more will be a loss for the user, which can be abused as described below.

Proof of Concept

• Suppose that there is only one depositor in the pool, who is Bob.
• Alice intends to deposit the same amount as Bob, so she does so and sets minLpTokenAmount to x.
• However, Bob quickly withdraws all his deposit, changes the values in the xy = k invariant since he starts from scratch.
• With the right choice of manipulation, it is easy for Bob to cause Alice to meet her minLpTokenAmount requirement of x, which forcing her to severely overpay.

Tools Used

Manual.

Recommended Mitigation Steps

Do something similar to the router in UniswapV2 where the tokens taken from the user are exactly the amount needed to both have the same ratio in the addQuote calculations.

Lines of code

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

Vulnerability details

Impact

In some protocols, a common vulnerability exists where the first depositor can be frontrun to have their entire initial deposit stolen (e.g. see https://github.com/code-423n4/2022-01-sherlock-findings/issues/39). If the first user does not properly set the minLpTokenAmount in add function, they will be susceptible to this attack. For extra safety, you could consider minting some of the initial Math.sqrt(x * y) tokens from the first deposit to the zero address.

Proof of Concept

• The first depositor deposits x baseToken and y fractionalToken, and sets their min minLpTokenAmount too low.
• An attacker sees this tx in the mempool and deposits 1 wei of x and 1 wei of y. This makes the LP total supply 1. The attack also manually transfers x+1 and y+1 baseToken and fractionalToken directly to the pool contract.
• The first depositor tx will happen, which will round the number of LP tokens they receive down to zero (or perhaps some other low number depending on the attacker's manual transfer). If they didn't set minLpTokenAmount high enough, then this would be an issue.

Tools Used

Manual.

Recommended Mitigation Steps

Consider minting some of the initial shares minted to the zero address, so that an attacker can't do this attack.

Lines of code

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

Vulnerability details

Impact

The buyQuote function calculates the "amount of base tokens required to buy a given amount of fractional tokens". This value is rounded down, which differs from the similar function in UniswapV2. Since the value is rounded down instead of up, the user will pay less than they need to, which can be abused to make a profit.

Proof of Concept

• Suppose baseTokens are valuable per wei, e.g. WBTC.
• The buyQuote calculations would have a relatively small amount of outputAmount token round down to zero, so that the user pays nothing to get a non-zero amount of output, which would be worth a non-trivial amount of money in the case of WBTC.
• The user can repeat multiple times and essentially extract free money from the protocol.

Tools Used

Manual.

Recommended Mitigation Steps

Add one to the final answer in buyQuote, similar to how getAmountIn works in UniswapV2.