sherlock-audit / 2024-08-cork-protocol-judging

2 stars 2 forks source link

Minato7namikazi - Inaccurate Redemption Preview for Expired depeg swaps with unliquidated LP #206

Closed sherlock-admin2 closed 1 month ago

sherlock-admin2 commented 1 month ago

Minato7namikazi

Medium

Inaccurate Redemption Preview for Expired depeg swaps with unliquidated LP

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L400

In the function: _tryLiquidateLpAndRedeemCtToPsm

The function doesn't account for the case where the Depeg Swap (DS) has expired but the LP hasn't been liquidated yet.

Here's the relevant part of the function:

function _tryLiquidateLpAndRedeemCtToPsm(State storage self, uint256 dsId, IDsFlashSwapCore flashSwapRouter)
    internal
    view
    returns (uint256 totalRa, uint256 pa)
{
    uint256 ammCtBalance;

    (totalRa, ammCtBalance) = __calculateTotalRaAndCtBalance(self, flashSwapRouter, dsId);

    uint256 reservedDs = flashSwapRouter.getLvReserve(self.info.toId(), dsId);

    // pair DS and CT to redeem RA
    totalRa += reservedDs > ammCtBalance ? ammCtBalance : reservedDs;

    uint256 raFromCt;
    // redeem CT to get RA + PA
    (pa, raFromCt) = PsmLibrary.previewRedeemWithCt(
        self,
        dsId,
        // CT attributed to PA
        reservedDs > ammCtBalance ? 0 : ammCtBalance - reservedDs
    );
}

The issue is that this function is used in the previewRedeemExpired function to simulate the liquidation of LP and redemption of CT to PSM. However, it doesn't check if the DS has expired and if the LP has been liquidated.

According to the litepaper, when a DS expires, the following should happen:

  1. The AMM LP is redeemed to receive CT + RA
  2. Any excess DS in the LV is paired with CT to redeem RA
  3. The excess CT is used to claim RA + PA in the PSM
  4. End state: Only RA + redeemed PA remains

The current implementation doesn't ensure that these steps are simulated correctly if the DS has expired but the LP hasn't been liquidated yet.

This missed case could lead to incorrect calculations when users try to preview their redemptions after a DS has expired but before the LP has been officially liquidated. It might show less RA and PA than what users would actually receive after the proper liquidation process.

Mitigation

  1. Check if the DS has expired
  2. If expired and LP not liquidated, simulate the full liquidation process as described in the litepaper
  3. Then proceed with the current calculations

This change would ensure that the preview accurately reflects what users would receive in all scenarios, including the post-expiry, pre-liquidation state.

Impact and PoC:

Let's say we have the following scenario:

Step 1: User calls previewRedeemExpiredLv

function previewRedeemExpiredLv(Id id, uint256 amount) external view {
    // ... (other code)
    (uint256 totalRa, uint256 pa) = _tryLiquidateLpAndRedeemCtToPsm(self, self.globalAssetIdx, flashSwapRouter);
    // ... (other code)
}

Step 2: Inside _tryLiquidateLpAndRedeemCtToPsm:

The result might be something like: totalRa = 150 pa = 50

Step 3: The actual redemption process in redeemExpiredLv:

function redeemExpiredLv(Id id, address receiver, uint256 amount, bytes memory rawLvPermitSig, uint256 deadline) external {
    // ... (other code)
    if (ds.isExpired() && !self.vault.lpLiquidated.get(dsId)) {
        _liquidatedLp(self, dsId, ammRouter, flashSwapRouter);
        assert(self.vault.balances.ra.locked == 0);
    }
    // ... (other code)
}

Here, _liquidatedLp is called, which performs the actual liquidation process as described in the litepaper.

The actual result might be slightly different, perhaps: totalRa = 155 pa = 45

The difference arises because the preview function doesn't perfectly simulate the complex liquidation process, especially the selling of excess CT for RA.

Conclusion:

The main consequence is that the preview function might provide inaccurate estimates in the specific scenario where a DS has expired but the LP hasn't been liquidated yet. This could lead to minor discrepancies between what users expect to receive based on the preview and what they actually receive upon redemption.

sherlock-admin4 commented 1 month ago

1 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Low severity. We have no impact here. Tha same impact like your other issue #209.