The malicious first liquidity provider can steal user's fund or prevent others from providing liquidity

[code423n4]

Lines of code

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

Vulnerability details

Impact

If a malicious user is the first person who is interacting with the pair, he can manipulate the reserve so that stealing fund from users if minLpTokenAmount == 0 or preventing users from adding liquidity if minLpTokenAmount != 0.

Proof of Concept

• Suppose a pair is just created.
• Bob (a malicious user) is the first person who is interacting with this pair.
• He gets some fractional tokens by wrapping his NFT.
  https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L217
• Then, Bob adds liquidity to the pair by calling the function add with the following parameters and providing the required baseToken which is USDC for example.
  • baseTokenAmount = 1
  • fractionalTokenAmount = 1
  • minLpTokenAmount = 1
    https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L63
• By doing so, 1 USDC and 1 fractional token will be transferred to the pair from Bob, and 1 lpToken will be minted to him based on the calculation done in the function addQuote. So, the totalSupply = 1.

function addQuote(uint256 baseTokenAmount, uint256 fractionalTokenAmount) public view returns (uint256) {
        uint256 lpTokenSupply = lpToken.totalSupply();
        if (lpTokenSupply > 0) {
            // calculate amount of lp tokens as a fraction of existing reserves
            uint256 baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves();
            uint256 fractionalTokenShare = (fractionalTokenAmount * lpTokenSupply) / fractionalTokenReserves();
            return Math.min(baseTokenShare, fractionalTokenShare);
        } else {
            // if there is no liquidity then init
            return Math.sqrt(baseTokenAmount * fractionalTokenAmount);
        }
    }

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

• Now Bob has the control of the pair and can apply two kinds of exploit:
  • First: if a user is going to add for example 1000 USDC and some fractional token with minLpTokenAmount parameter equal to zero, Bob front-runs the user and transfers directly to the pair 1000 USDC. By doing so, baseTokenReserves() = 1 + 1000 and total lpToken minted to the user will be equal to zero because of 1000 * 1 /(1 + 1000) = 0:
    uint256 baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves();
    https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L421
    Although, the user provided 1000 USDC, but zero lpToken is minted for the user. So, totalSupply = 1 and baseTokenReserves = 1 + 1000 + 1000. Now, Bob can remove his liquidity by burning his only lpToken, and receive 2001 USDC (1001 belongs to Bob, and 1000 USDC is user's fund). So, Bob could steal 1000 USDC from the user.
  • Second: if a user is going to add for example 1000 USDC and some fractional token with minLpTokenAmount parameter equal to nonzero X, Bob front-runs the user and transfers directly to the pair 1000/X USDC. By doing so, total lpToken minted to the user will be less than X because 1000 * 1 /(1 + 1000/X) = 1000 * X / (X + 1000) < X:
    uint256 baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves();
    https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L421
    As a result the user's transaction will be reverted because of:
    require(lpTokenAmount >= minLpTokenAmount, "Slippage: lp token amount out");
    https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L80
    So, Bob could prevent any user from adding liquidity with nonzero minLpTokenAmount . Now, Bob can remove his liquidity by burning his only lpToken, and receive 1 + 1000/X USDC that belongs to him.

Tools Used

Recommended Mitigation Steps

• First Solution: The first person who is adding liquidity must add at least 100 USDC. So, totalSupply = 100 and baseTokenReserves() =100. By doing so, Bob (as the first person) must add 100 USDC and in return he receives 100 lpToken. To apply the attack when a user is adding 1000 USDC, Bob must transfer directly 99.9k + 1 USDC to be able to steal the user's 1000 USDC, because 1000 * 100 / (100 + Bob's direct transfer). So, Bob's attack requires a large amount of fund which makes it financially unreasonable.

function addQuote(uint256 baseTokenAmount, uint256 fractionalTokenAmount) public view returns (uint256) {
        uint256 lpTokenSupply = lpToken.totalSupply();
        if (lpTokenSupply > 0) {
            // calculate amount of lp tokens as a fraction of existing reserves
            uint256 baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves();
            uint256 fractionalTokenShare = (fractionalTokenAmount * lpTokenSupply) / fractionalTokenReserves();
            return Math.min(baseTokenShare, fractionalTokenShare);
        } else {
            require(baseTokenAmount >= 100 * 10**6);
            // if there is no liquidity then init
            return Math.sqrt(baseTokenAmount * fractionalTokenAmount);
        }
    }

• Second Solution: It is recommended that when the pair is created a large amount of lpToken be minted to the pair. For example, if the protocol mints 1000 lpToken to the pair, Bob must provide almost 1,001,000 USDC to make this 1000 * (1000 + 1)/ (1 + Bob's direct transfer) equal to zero (after rounded down by EVM), which is almost impossible for Bob.

[c4-judge]

berndartmueller marked the issue as duplicate of #442

[c4-judge]

berndartmueller marked the issue as satisfactory