Minato7namikazi - Risk of Misleading Users in `previewRedeemExpiredLv` function logic

[sherlock-admin2]

Minato7namikazi

Medium

Risk of Misleading Users in previewRedeemExpiredLv function logic

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/core/Vault.sol#L149

in the function previewRedeemExpiredLv

the function doesn't account for the case where the DS (Depeg Swap) has expired, but the LP (Liquidity Pool) hasn't been liquidated yet. This can lead to incorrect calculations and potentially misleading preview results.

function previewRedeemExpiredLv(Id id, uint256 amount)
    external
    view
    override
    LVWithdrawalNotPaused(id)
    returns (uint256 attributedRa, uint256 attributedPa, uint256 approvedAmount)
{
    State storage state = states[id];
    (attributedRa, attributedPa, approvedAmount) = state.previewRedeemExpired(amount, _msgSender(), getRouterCore());
}

The issue is that this function relies on the previewRedeemExpired function in the VaultLibrary, which doesn't properly handle the case where the DS has expired but the LP hasn't been liquidated yet.

In the actual redeemExpiredLv function, there's a check and a call to _liquidatedLp if the DS has expired but the LP hasn't been liquidated:

if (ds.isExpired() && !self.vault.lpLiquidated.get(dsId)) {
    _liquidatedLp(self, dsId, ammRouter, flashSwapRouter);
    assert(self.vault.balances.ra.locked == 0);
}

this logic is missing in the preview function. This can lead to inconsistencies between the preview and the actual redemption, potentially causing issues for users relying on the preview function to make decisions.

The impact of this vulnerability is that users might receive incorrect estimates when previewing their expired LV redemptions. Leading to funds losses because they might decide wrong decisions

To fix this, the previewRedeemExpiredLv function should simulate the LP liquidation process when the DS has expired but the LP hasn't been liquidated yet, similar to what's done in the actual redemption function. This would provide more accurate previews that align with the actual redemption process.

Trigger Condition:

The bug can be triggered when all of the following conditions are met :

a) The DS (Depeg Swap) has expired. b) The LP (Liquidity Pool) hasn't been liquidated yet. c) A user calls previewRedeemExpiredLv before anyone has called redeemExpiredLv.

Why it can be triggered:

The previewRedeemExpiredLv function doesn't simulate the LP liquidation process, while the actual redeemExpiredLv function does

Flow/PoC Example:

Let's consider a scenario:

Initial state :

- DS has expired
- LP hasn't been liquidated
- LV (Liquidity Vault) contains:
  100 RA in the AMM pool
  50 PA in the AMM pool
  20 RA in the withdrawal pool
  10 PA in the withdrawal pool
- User wants to redeem 1000 LV tokens (assume total supply is 10,000 LV)

Step 1 :  User calls previewRedeemExpiredLv

Result:
- attributedRa ≈ 2 (20 * 1000 / 10000)
- attributedPa ≈ 1 (10 * 1000 / 10000)

Step 2: User calls redeemExpiredLv

Actual Result:

- LP is liquidated first
- Total available: 120 RA, 60 PA
- attributedRa ≈ 12 (120 * 1000 / 10000)
- attributedPa ≈ 6 (60 * 1000 / 10000)

As we can see, the actual redemption yields significantly more assets than the preview suggested.

To fix this, the previewRedeemExpiredLv function should simulate the LP liquidation process when necessary, similar to how redeemExpiredLv does. This would provide more accurate previews that align with the actual redemption process.

[sherlock-admin4]

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

tsvetanovv commented:

Low severity. We have no impact here. The purpose of previewRedeemExpiredLv() is just for simulation.