Cheerful Taffy Dolphin - Vault Collateral Over-Reservation Bug Leads to 100% Capital Lockup Due to Zero-State Division Edge Case

[sherlock-admin4]

Cheerful Taffy Dolphin

Medium

Vault Collateral Over-Reservation Bug Leads to 100% Capital Lockup Due to Zero-State Division Edge Case

Summary

In the vault's allocation logic, there's a critical bug in how ineligible collateral is calculated during early vault states or after full redemptions. The issue stems from the _ineligible() function that determines how much collateral should be reserved vs made available for new allocations:

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/vault/contracts/Vault.sol#L459

function _ineligible(Context memory context, UFixed6 deposit, UFixed6 withdrawal) private pure returns (UFixed6) {
    UFixed6 redemptionEligible = UFixed6Lib.unsafeFrom(context.totalCollateral)
        .unsafeSub(context.global.assets.add(withdrawal))
        .unsafeSub(context.global.deposit.sub(deposit));

    return redemptionEligible.mul(
        context.global.redemption.unsafeDiv(
            context.global.shares.add(context.global.redemption)
        )
    ).add(context.global.assets);
}

The bug emerges in the edge case when there are no shares and no redemptions in the vault - a state that occurs at vault initialization or if all shares have been redeemed.

// In UFixed6Lib:
function unsafeDiv(UFixed6 a, UFixed6 b) internal pure returns (UFixed6) {
    if (isZero(b)) {
        return isZero(a) ? ONE : MAX;
    } else {
        return div(a, b);
    }
}

This is used in the ineligible calculation:

redemptionEligible.mul(
    context.global.redemption.unsafeDiv(
        context.global.shares.add(context.global.redemption)
    )
)

Consider what happens when the vault has no shares and no redemptions:

• context.global.shares is 0
• context.global.redemption is 0
• So context.global.shares.add(context.global.redemption) is 0
• And context.global.redemption is also 0

In this case, we hit the 0/0 condition in unsafeDiv, which returns ONE (1e6 in Fixed6 representation). This means:

• redemptionEligible.mul(ONE) equals redemptionEligible
• So we're incorrectly marking the full redemptionEligible amount as ineligible for allocation
• When in reality, with no shares and no redemptions, none of the collateral should be reserved for redemptions

The current fallback to ONE effectively reserves 100% of redeemable collateral even when there are no redemptions pending, which is overly conservative and could unnecessarily restrict capital efficiency.

Fix

A more capital efficient approach would be to explicitly handle this edge case:

function _ineligible(Context memory context, UFixed6 deposit, UFixed6 withdrawal) private pure returns (UFixed6) {
    UFixed6 redemptionEligible = UFixed6Lib.unsafeFrom(context.totalCollateral)
        .unsafeSub(context.global.assets.add(withdrawal))
        .unsafeSub(context.global.deposit.sub(deposit));
        
    // If there are no redemptions, don't reserve any collateral regardless of shares
    if (context.global.redemption.isZero()) return context.global.assets;
    
    return redemptionEligible
        .mul(context.global.redemption)
        .unsafeDiv(context.global.shares.add(context.global.redemption))
        .add(context.global.assets);
}