Inability to Fully Withdraw Liquidity or Liquidate Last LP left or only LP in Pool

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L80 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Liquidity.sol#L157 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L140 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L170

Vulnerability details

Summary:

The protocol's current mechanism for liquidity withdrawal does not allow the last liquidity provider left or only LP in a pool to fully withdraw their stake or be liquidated due to a reserve threshold constraint.

Vulnerability Details:

LPs withdraw liquidity using withdrawCollateralAndClaim or withdrawLiquidityAndClaim functions, both relying on removeLiquidity. The removeLiquidity function includes a requirement preventing reserve balances from dropping below PoolUtils.DUST (100). This restriction implies that the initial or final LP can't fully withdraw their liquidity or rewards. While the amount will be negligible for tokens like ETH (0.0000000000000001) with 18 decimal places, it could be more substantial for tokens with fewer decimals.

function removeLiquidity(
        ...
    ) external nonReentrant returns (uint256 reclaimedA, uint256 reclaimedB) {
        ...
        require(
            (reserves.reserve0 >= PoolUtils.DUST) && (reserves.reserve0 >= PoolUtils.DUST),
            "Insufficient reserves after liquidity removal"
        );
    ...
    }

More importantly, the liquidateUser function utilized for liquidating undercollateralized positions withdraws all of a user's liquidity. In scenarios where a user is the sole LP, the PoolUtils.DUST constraint prevents a liquidation even if a position is undercollateralized.

function liquidateUser(address wallet) external nonReentrant {
        require(wallet != msg.sender, "Cannot liquidate self");

        // First, make sure that the user's collateral ratio is below the required level
        require(canUserBeLiquidated(wallet), "User cannot be liquidated");

        uint256 userCollateralAmount = userShareForPool(wallet, collateralPoolID);

        // Withdraw the liquidated collateral from the liquidity pool.
        // The liquidity is owned by this contract so when it is withdrawn it will be reclaimed by this contract.
        (uint256 reclaimedWBTC, uint256 reclaimedWETH) =
            pools.removeLiquidity(wbtc, weth, userCollateralAmount, 0, 0, totalShares[collateralPoolID]);
         ...
    }

While this edge case is unlikely, it should still be handled by the protocol as it can not only lead to a loss of user funds but also be used to abuse the liquidation system and block a position from being liquidated.

Impact:

This issue restricts the last LP left or only LP in a pool from fully withdrawing their liquidity or rewards, leading to a loss of funds. It also presents a loophole in the liquidation process.

Proof Of Concept

function testLPCantFW() public {
    bytes32 poolID1 = PoolUtils._poolID( wbtc, weth );
    bytes32[] memory poolIDs = new bytes32[](1);
    poolIDs[0] = poolID1;
    skip(2 days);
    // Total needs to be worth at least $2500
    uint256 depositedWBTC = ( 1000 ether *10**8) / priceAggregator.getPriceBTC();
    uint256 depositedWETH = ( 1000 ether *10**18) / priceAggregator.getPriceETH();
    (uint256 reserveWBTC, uint256 reserveWETH) = pools.getPoolReserves(wbtc, weth);
    vm.startPrank(alice);
    // Alice will deposit collateral 
    (uint256 addedAmountWBTC, uint256 addedAmountWETH, uint256 addedLiquidity) = collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );
    // Alice tries to remove all collateral
    vm.expectRevert("Insufficient reserves after liquidity removal");
    collateralAndLiquidity.withdrawCollateralAndClaim(addedLiquidity ,0,0,block.timestamp);
    vm.stopPrank();
    }

Tools Used:

• Manual analysis
• Foundry

Recommendation:

The team should deposit a minimal amount of liquidity (slightly above the PoolUtils.DUST threshold) to all current and future whitelisted pools. This ensures that the reserve threshold does not hinder full withdrawal or liquidation processes.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #459

[c4-judge]

Picodes marked the issue as satisfactory