code-423n4 / 2024-01-salty-findings

5 stars 3 forks source link

Wrong comparison to give added liquidity always uses based on WETH reserves and never WBTC #936

Closed c4-bot-10 closed 5 months ago

c4-bot-10 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L131-L135

Vulnerability details

Impact

The Liquidity tokens to give for providing liquidity always calculates added Liquidity based on WETH instead of WBTC, this means if WBTC reserves get low, there's no means for code to prioritize WBTC liquidity

Proof of Concept

In _addLiquidity

        if ( addedAmount0 > addedAmount1)
            //@audit this comparision doesn't take decimals into account or their relative ratios, this is wrong
            addedLiquidity = (totalLiquidity * addedAmount0) / reserve0;            
        else
            addedLiquidity = (totalLiquidity * addedAmount1) / reserve1;

The reason addedLiquidity will only be computed by WETH is because, WBTC : WETH pool value is in 1:20. That is, WBTC is costlier than WETH and an amount x needs lower WBTC than it does for WETH. The check

if ( addedAmount0 > addedAmount1)

doesn't take this into account.

Tools Used

Manual analysis

Recommended Mitigation Steps

Uniswap's way of checking liquidity may help circumvent this issue

..

        } else {
            liquidity = Math.min(amount0.mul(_totalSupply) / _reserve0, amount1.mul(_totalSupply) / _reserve1);
        }

Assessed type

Uniswap

c4-judge commented 5 months ago

Picodes marked the issue as unsatisfactory: Insufficient proof