Potential loss of funds due to inflexible liquidation process in CDPVault

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/CDPVault.sol#L579

Vulnerability details

Impact

The liquidatePositionBadDebt function in the CDPVault contract has a critical flaw that can lead to failed liquidations or loss of funds for liquidators. The function requires the liquidator to provide an exact amount of repayment, which is impractical in real-world scenarios due to potential price fluctuations between transaction submission and execution. If the liquidator sends more than the required amount, the excess is effectively lost as there's no refund mechanism.

This issue can result in:

1. Failed liquidations if the collateral value changes slightly.
2. Loss of funds for liquidators if they overpay.
3. Reduced willingness of liquidators to participate, potentially destabilizing the system.

Proof of Concept

The problematic code is in the liquidatePositionBadDebt function:

liquidatePositionBadDebt

function liquidatePositionBadDebt(address owner, uint256 repayAmount) external whenNotPaused {
    // ... (some code omitted for brevity)
    
    uint256 discountedPrice = wmul(spotPrice_, liqConfig_.liquidationDiscount);
    if (calcTotalDebt(debtData) <= wmul(position.collateral, discountedPrice)) revert CDPVault__noBadDebt();
    
    uint256 takeCollateral = wdiv(repayAmount, discountedPrice);
    if (takeCollateral < position.collateral) revert CDPVault__repayAmountNotEnough();

    takeCollateral = position.collateral;
    repayAmount = wmul(takeCollateral, discountedPrice);
    // ... (rest of the function)
}

The issue is done in

if (takeCollateral < position.collateral) revert CDPVault__repayAmountNotEnough();

because:

• The function calculates takeCollateral based on the repayAmount provided by the liquidator.
• It then checks if takeCollateral is less than the position's collateral, reverting if true.
• If the check passes, it sets takeCollateral to the position's full collateral amount.
  Finally, it recalculates repayAmount based on the full collateral.

This means that the liquidator must provide a repayAmount that, when divided by the discounted price, results in a value greater than or equal to the position's collateral. Any excess amount sent by the liquidator is effectively lost.

Tools Used

Manual analysis

Recommended Mitigation Steps

• Modify the function to accept a maximum repay amount from the liquidator, this is just an examlpe, please adjust it to fit into the code correctly.

function liquidatePositionBadDebt(address owner, uint256 maxRepayAmount) external whenNotPaused {
    // ... (existing code)
    
    uint256 requiredRepayAmount = wmul(position.collateral, discountedPrice);
    if (maxRepayAmount < requiredRepayAmount) revert CDPVault__repayAmountNotEnough();
    
    uint256 actualRepayAmount = requiredRepayAmount;
    uint256 refund = maxRepayAmount - actualRepayAmount;
    
    // Use actualRepayAmount for further calculations and transfers
    
    // Refund any excess to the liquidator
    if (refund > 0) {
        underlyingToken.safeTransfer(msg.sender, refund);
    }
    
    // ... (rest of the function)
}

• Consider implementing a slippage protection mechanism to allow liquidators to specify a maximum acceptable collateral amount or minimum acceptable repay amount.

Assessed type

Other

[evokid]

Hey Judge @koolexcrypto, I believe this issue a duplicate of issue code-423n4/2024-07-loopfi-findings#62, could please check it out.