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

11 stars 6 forks source link

Typo can lead to one of the pool's reserves to reach 0 and skew the ratio #822

Closed c4-bot-3 closed 9 months ago

c4-bot-3 commented 9 months ago

Lines of code

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

Vulnerability details

Impact

One token's reserves in a pool can hit 0. Leading to skewed reserve ratio

Proof of Concept

A typing error in pools.sol's remove liquidity function leads to the contract performing both the DUST amt checks on reserves.reserve0. Which can lead to one of the pools emptying out and skewing the reserve ratio.

        function removeLiquidity( IERC20 tokenA, IERC20 tokenB, uint256 liquidityToRemove, uint256 minReclaimedA, uint256 minReclaimedB, uint256 totalLiquidity ) external nonReentrant returns (uint256 reclaimedA, uint256 reclaimedB)
        {

        ...

        require((reserves.reserve0 >= PoolUtils.DUST) && (reserves.reserve0 >= PoolUtils.DUST), "Insufficient reserves after liquidity removal");
        ///@audit ^ both the DUST checks are on reserve0

        ...

        emit LiquidityRemoved(tokenA, tokenB, reclaimedA, reclaimedB, liquidityToRemove);
        }

Tools Used

Manual Review

Recommended Mitigation Steps

The bug seems to have made its way into the codebase during ABDK's fix code's review commit. In the 2nd check make sure to check reserves.reserve1 >= DUST

Assessed type

Error

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #1041

c4-judge commented 8 months ago

Picodes changed the severity to 2 (Med Risk)

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory