<!-- canonical: https://0xsimao.com/findings/cork-protocol-withdrawing-expiry-lost-funds -->

# Withdrawing all `lv` before expiry will lead to lost funds in the Vault

Medium · Sherlock · Derivatives · 29th August 2024

Finding M-5 of the Cork Protocol competition.

- Protocol: https://audits.sherlock.xyz/contests/506
- Codebase: https://github.com/0xsimao/2024-08-cork-protocol/tree/db23bf67e45781b00ee6de5f6f23e621af16bd7e
- Source: https://github.com/sherlock-audit/2024-08-cork-protocol-judging/issues/211

---

### Summary

[VaultLib:redeemEarly()](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L639) redeems users' liquidity vault positions, `lv` for `Ra`, [before](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L649) expiry. After expiry, it is not possible to deposit into the vault or redeem early.

Whenever all users redeem early, when it gets to the expiry date, [VaultLib::_liquidatedLp()](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L349) is called to remove the lp position from the `AMM` into `Ra` and `Ct` (redeem from the `PSM` into more `Ra` and `Pa`) and split among all `lv` holders.

However, as the total supply of `lv` is `0` due to users having redeemed all their positions via `VaultLib::redeemEarly()`, when it gets to [VaultPoolLib::reserve()](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L392), it [reverts](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultPoolLib.sol#L19) due to a division by `0` [error](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/MathHelper.sol#L134), never allowing the `Vault::_liquidatedLp()` call to go through. 

As the `Ds` has expired, it is also not possible to deposit into it to increase the `lv` supply, so all funds are forever stuck.

### Root Cause

In `MathHelper:134`, the `ratePerLv` reverts due to division by 0. It should calculate the rate after the return guard that checks if the `totalLvIssued == 0`.

### Internal pre-conditions

1. All users must redeem early.

### External pre-conditions

None.

### Attack Path

1. All users redeem early via `Vault::redeemEarlyLv()`.
2. The `Ds` expires and there is no `lv` tokens, making all funds stuck.

### Impact

All funds are stuck.

### PoC

The `MathHelper` separates liquidity by calculating first the `ratePerLv`, which will trigger a division by 0 revert.
```solidity
function separateLiquidity(uint256 totalAmount, uint256 totalLvIssued, uint256 totalLvWithdrawn)
    external
    pure
    returns (uint256 attributedWithdrawal, uint256 attributedAmm, uint256 ratePerLv)
{
    // with 1e18 precision
    ratePerLv = ((totalAmount * 1e18) / totalLvIssued);

    // attribute all to AMM if no lv issued or withdrawn
    if (totalLvIssued == 0 || totalLvWithdrawn == 0) {
        return (0, totalAmount, ratePerLv);
    }
    ...
}
```

### Mitigation

The `MathHelper` should place the return guard first:
```solidity
function separateLiquidity(uint256 totalAmount, uint256 totalLvIssued, uint256 totalLvWithdrawn)
    external
    pure
    returns (uint256 attributedWithdrawal, uint256 attributedAmm, uint256 ratePerLv)
{
    // attribute all to AMM if no lv issued or withdrawn
    if (totalLvIssued == 0 || totalLvWithdrawn == 0) {
        return (0, totalAmount, 0);
    }
    ...
    // with 1e18 precision
    ratePerLv = ((totalAmount * 1e18) / totalLvIssued);
}
```

---

Related findings:

- [`totalEarnings` is incorrect when withdrawing after ending which will withdraw too many funds leaving the `Vault` insolvent](https://0xsimao.com/findings/saffron-lido-vaults-ending-withdraw-leaving-insolvent): Saffron Lido Vaults
- [DarkpoolAssetManager::Split() into 2 equal amounts leads to lost funds](https://0xsimao.com/findings/singularity-darkpool-split-equal-amounts): Singularity
- [Lost withdrawals due to `ManagedLeveragedVault::openDen()` using all `asset()` balance](https://0xsimao.com/findings/beraborrow-i-lost-withdrawals-managed-den): Beraborrow Managed Dens
- [Changing providers might lead to lost assets in BorrowingVault and YieldVault](https://0xsimao.com/findings/fuji-finance-changing-providers-lost-yield): Fuji Finance