Cheerful Taffy Dolphin - Vault Profit Share Bypass During Migration Results in Permanent Revenue Loss Through Zero-Mark Condition

[sherlock-admin2]

Cheerful Taffy Dolphin

Medium

Vault Profit Share Bypass During Migration Results in Permanent Revenue Loss Through Zero-Mark Condition

Summary

A critical vulnerability exists in the _calculateProfitShare function where profit shares are not allocated when the high-water mark (mark) is zero, resulting in potential economic loss during vault migrations.`

When the mark parameter is zero (typical during vault migration), the function prematurely returns after calculating newMark but before computing profit shares. This bypasses the profit share calculation entirely, even though there may be legitimate profits to distribute.

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

function _calculateProfitShare(
    Checkpoint memory self,
    UFixed18 mark,
    VaultParameter memory parameter
) private pure returns (UFixed18 newMark, UFixed6 profitShares) {
    if (self.shares.isZero()) return (UFixed18Lib.ONE, UFixed6Lib.ZERO);
    
    newMark = mark.max(UFixed18Lib.from(UFixed6Lib.unsafeFrom(self.assets))
             .div(UFixed18Lib.from(self.shares)));
    
    // Bug: Early return skips profit share calculation
    if (mark.isZero()) return (newMark, UFixed6Lib.ZERO);
    
    // Never reaches this code when mark is zero
    UFixed6 profitAssets = parameter.profitShare
        .mul(UFixed6Lib.from(newMark.sub(mark).mul(UFixed18Lib.from(self.shares))));
}

Scenario

Initial State:
- self.shares = 1000
- self.assets = 1500
- parameter.profitShare = 0.2 (20%)
- mark = 0

Expected Calculation:
1. newMark = 1.5
2. profitAssets = 0.2 * (1.5 - 0) * 1000 = 300
3. profitShares = (300 * 1000) / (1500 - 300) = 250

Actual Result:
- Returns (1.5, 0)
- 250 profit shares worth 300 assets are not allocated

Impact

The zero-mark condition triggers a complete bypass of profit share calculation during migration, nullifying the coordinator's claim to a 20% share of vault performance. When initial depositors enter at mark=0, their returns above the zero-mark are excluded from profit share computations, compounding into a permanent revenue loss at the protocol level. This calculation bypass effectively breaks the vault's core economic incentive mechanism by allowing value accrual without corresponding profit share minting.

Proof of Concept

function testMigrationProfitShareBug() public {
    Checkpoint memory checkpoint;
    checkpoint.shares = UFixed6.wrap(1000e6);
    checkpoint.assets = Fixed6.wrap(1500e6);
    
    VaultParameter memory param;
    param.profitShare = UFixed6.wrap(0.2e6); // 20%
    
    UFixed18 mark = UFixed18.wrap(0);
    
    (UFixed18 newMark, UFixed6 profitShares) = 
        checkpoint._calculateProfitShare(mark, param);
        
    // profitShares is 0 when it should be ~250e6
    assert(profitShares.isZero());
}

Recommended Fix

function _calculateProfitShare(
    Checkpoint memory self,
    UFixed18 mark,
    VaultParameter memory parameter
) private pure returns (UFixed18 newMark, UFixed6 profitShares) {
    if (self.shares.isZero()) return (UFixed18Lib.ONE, UFixed6Lib.ZERO);

    newMark = mark.max(UFixed18Lib.from(UFixed6Lib.unsafeFrom(self.assets))
             .div(UFixed18Lib.from(self.shares)));

    // Calculate profits even when mark is zero
    UFixed6 profitAssets = parameter.profitShare
        .mul(UFixed6Lib.from(newMark.sub(mark).mul(UFixed18Lib.from(self.shares))));

    if (UFixed6Lib.unsafeFrom(self.assets).sub(profitAssets).isZero()) 
        return (newMark, UFixed6Lib.ZERO);

    profitShares = profitAssets.mul(self.shares)
        .div(UFixed6Lib.unsafeFrom(self.assets).sub(profitAssets));
    
    return (newMark, profitShares);
}