make token per liquidity very high by direct early token transfer to Pair contract which would cause users to lose funds when they provide liquidity

[code423n4]

Lines of code

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

Vulnerability details

Impact

Function addQuote() calculates the amount of LP tokens received for adding a given amount of base tokens and fractional tokens. attacker can manipulate token per liquidity value and make it so high that whenever users wants to provide liquidity they lose a lot of funds because of the rounding. by doing this attacker can steal other users funds and make Pair contract to be useless.

Proof of Concept

This is addQuote() code:

    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);
        }
    }

As you can see the lp token amount is calculated as a share of existing deposits. If there are no existing deposits, then initializes to sqrt(baseTokenAmount * fractionalTokenAmount). and in the function add() this amount is used to mint lp token for user.

    function add(uint256 baseTokenAmount, uint256 fractionalTokenAmount, uint256 minLpTokenAmount)
        public
        payable
        returns (uint256 lpTokenAmount)
    {
.....
.....
        // calculate the lp token shares to mint
        lpTokenAmount = addQuote(baseTokenAmount, fractionalTokenAmount);
.....
.....
.....
        // mint lp tokens to sender
        lpToken.mint(msg.sender, lpTokenAmount);
....
....
    }

So to exploit this attacker can perform this steps:

1. create a Pair for NFT1 and BaseToken1 and merkle root hash as 0x0.
2. wrap 10 NFT token for 10 * 1e18 fractional token.
3. first deposit 1 fractional token and 1 baseToken as liquidity with function add() and contract would mint 1 LP token for attacker.
4. directly transfer rest of 10 * 1e18 fractional token to contract address and 1000 * 1e18 - 1 amount of baseToken to contract address.
5. now lpToken.totalSupply() is 1, baseTokenReserves() is 1000 * 1e18, fractionalTokenReserves() is 10 * 10e18.
6. user1 wants to deposit 1600 * 1e18 baseToken and 16 * 10e18 fractional token as liquidity and he would call add() for this.
7. contract in addQuote() would calculate lp token amount as: baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves() = 1500 * 10e18 * 1 / 1000 * 1e18 = 1, fractionalTokenShare = (fractionalTokenAmount * lpTokenSupply) / fractionalTokenReserves() = 16 * 1e18 * 1 / 10 * 10e18 = 1 and lpTokenAmount = Math.min(baseTokenShare, fractionalTokenShare) = min(1,1) =1 and contract would mint 1 lp token for user and transfer 1600 * 1e18 baseToken and 16 * 10e18 fractional token from user.
8. now lpToken.totalSupply() is 2, baseTokenReserves() is 2600 * 1e18, fractionalTokenReserves() is 26 * 10e18.
9. user1 would remove his liquidity by calling remove(1) and contract would transfer 50% of the liquidity to user (because user1 has 1 LP token of the total 2 LP tokens) (calculations are done in the removeQuote()) and user1 would receive 1300 * 1e18 baseToken and 13 * 10e18 fractional token.
10. if attacker remove his liquidity he would receive 1300 * 1e18 baseToken and 13 * 10e18 fractional token. and as you can see user1 add token and removed them and lost 300 * 1e18 baseToken and 3 * 10e18 fractional token immediately and attacker took them.

even if user1 wants to specify minLpTokenAmount when calling add() function it would cause whole transaction to fail. the problem is that attacker manipulated token per share value and make it so high that users would lose their liquidity deposits through rounding error in calculations. the numbers in the POC was just an example and scenario can be different and the attack would still be effective.
the early direct transfer of funds can be done by attacker with smart contract (which create Pair and add liquidity and transfer funds in one transaction) for popular NFT collection and merkle root 0x0 and popular baseTokens (like ETH) so attacker can perform the attack for common Pairs contract. also attacker doesn't need to be the first liquidity provider and if the first liquidity provider provide small amount of tokens attacker can directly transfer the tokens and perform the attack.

Tools Used

VIM

Recommended Mitigation Steps

one simple and fast solution would be add some precision for LP token amount so that token per liquidity can't be manipulate so easily.

[c4-judge]

berndartmueller marked the issue as duplicate of #442

[c4-judge]

berndartmueller marked the issue as satisfactory