0x73696d616f - Withdrawing all lv before expiry will lead to lost funds in the Vault

[sherlock-admin2]

0x73696d616f

Medium

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

Summary

VaultLib:redeemEarly() redeems users' liquidity vault positions, lv for Ra, before 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() 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(), it reverts due to a division by 0 error, 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.

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:

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);
}

[sherlock-admin2]

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

tsvetanovv commented:

Borderline Low/Medium

[ziankork]

This is valid. will fix

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
Cork-Technology/Depeg-swap#80