Cheerful Taffy Dolphin - Stale Parameter Usage in Rebalance Causes Incorrect Position Sizing and Risk Management Failures

[sherlock-admin3]

Cheerful Taffy Dolphin

Medium

Stale Parameter Usage in Rebalance Causes Incorrect Position Sizing and Risk Management Failures

Summary

The Vault contract contains a significant timing vulnerability in its parameter update mechanism that could result in incorrect market positions, financial loss, and state inconsistency. When updating vault parameters (maxDeposit, minDeposit, profitShare), the rebalancing operation executes using outdated parameters before the new values are stored, leading to incorrect position sizing and potential violations of intended risk constraints.

Parameter Update:

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

function updateParameter(VaultParameter memory newParameter) external onlyOwner {
    rebalance(address(0));  // Critical timing issue here
    _updateParameter(newParameter);
}

Context Loading with Old Parameters:

function _loadContext(address account) private view returns (Context memory context) {
    context.parameter = _parameter.read();  // Loads old parameters
    // ... loads rest of context
}

Parameter Usage in Core Operations:

function _maxDeposit(Context memory context) private view returns (UFixed6) {
    return context.latestCheckpoint.unhealthy() ?
        UFixed6Lib.ZERO :
        context.parameter.maxDeposit.unsafeSub(UFixed6Lib.unsafeFrom(totalAssets()).add(context.global.deposit));
}

The issue is that rebalancing executes a full market position adjustment using the old parameter values. This means:

1. The maxDeposit calculation during rebalance uses outdated deposit limits
2. The _strategy() function (which is called by _manage() during rebalance) bases its allocation decisions on old parameters
3. If the new parameters significantly change deposit limits or other constraints, the rebalance could set positions that immediately violate the new parameters
4. Market positions set during rebalance might need immediate readjustment after the parameter update, causing unnecessary gas costs and potential market impact

This is particularly problematic in scenarios where parameter updates are specifically intended to adjust risk parameters or position limits in response to market conditions.

Impact

The parameter update timing issue creates critical failure modes in the vault's market management. The vault executes rebalancing operations using stale parameter values, which manifests in multiple levels of technical failures:

At the position management level, all market orders during rebalance are sized according to outdated maxDeposit and strategy parameters, leading to incorrect leverage ratios and position sizes that violate the intended new risk parameters. For instance, if maxDeposit is being lowered from 1000 to 500, the rebalance will still execute positions sized for the 1000 limit.

These incorrectly sized positions trigger state inconsistencies where the vault's market exposure directly conflicts with its stored parameters. This forces additional transactions to realign positions, introducing unnecessary gas costs and potential sandwich attack vectors during the corrective operations.

This breaks the vault's risk management guarantees. When parameter updates are made in response to market conditions (e.g., reducing maxDeposit during high volatility), the delay in parameter application means the vault continues to operate at higher risk levels during the rebalance - precisely when risk management is most crucial.

Fix

Modify the updateParameter function to handle parameters before rebalancing:

function updateParameter(VaultParameter memory newParameter) external onlyOwner {
    // Store new parameters first
    _updateParameter(newParameter);
    
    // Rebalance with new parameters
    rebalance(address(0));
}

Ensure state consistency in _loadContext:

function _loadContext(address account) private view returns (Context memory context) {
    // Load latest parameters first to ensure all subsequent calculations use new values
    context.parameter = _parameter.read();
    
    // Load market state
    context.latestTimestamp = type(uint256).max;
    context.currentTimestamp = type(uint256).max;
    context.registrations = new Registration[](totalMarkets);
    context.collaterals = new Fixed6[](totalMarkets);

    // ... rest of context loading
}

And add validation in _updateParameter:

function _updateParameter(VaultParameter memory newParameter) private {
    // Validate parameters
    VaultParameter memory oldParameter = _parameter.read();
    
    // If reducing maxDeposit, verify current deposits don't exceed new limit
    if (newParameter.maxDeposit.lt(oldParameter.maxDeposit)) {
        if (UFixed6Lib.unsafeFrom(totalAssets()).gt(newParameter.maxDeposit)) 
            revert VaultParameterStorageInvalidError();
    }
    
    // Store and emit
    _parameter.store(newParameter);
    emit ParameterUpdated(newParameter);
}

The rebalance will now use the new parameters for all calculations and position adjustments, ensuring proper risk management and position sizing.