Cheerful Taffy Dolphin - Missing Position Size Validation in updateLeverage() Causes Forced Position Adjustments Across Pooled Vault Assets

[sherlock-admin3]

Cheerful Taffy Dolphin

Medium

Missing Position Size Validation in updateLeverage() Causes Forced Position Adjustments Across Pooled Vault Assets

Description

The updateLeverage function in Vault.sol calls rebalance(address(0)) but does not validate whether positions would remain valid under the new leverage settings. While rebalance settles current positions, it does not verify if those positions would be safely maintainable under the new leverage value.

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

// Vault.sol
function updateLeverage(uint256 marketId, UFixed6 newLeverage) external onlyOwner {
    rebalance(address(0));  // Settles but doesn't validate future position validity
    if (marketId >= totalMarkets) revert VaultMarketDoesNotExistError();
    _updateMarket(marketId, UFixed6Lib.MAX, newLeverage);
}

// MakerStrategyLib.sol
function _allocateMarket(
    MarketMakerStrategyContext memory marketContext,
    UFixed6 totalMargin,
    UFixed6 collateral,
    UFixed6 assets
) private pure returns (Target memory target, UFixed6 marketCollateral) {
    // Position calculations depend on leverage
    UFixed6 minAssets = marketContext.riskParameter.minMargin
        .unsafeDiv(marketContext.registration.leverage.mul(marketContext.riskParameter.maintenance));
    // ...
}

Impact

When a new leverage value is set, the positions held in marketContext.currentAccountPosition are immediately subjected to different minAssets calculations in _allocateMarket(), where newMinAssets = marketContext.riskParameter.minMargin.unsafeDiv(newLeverage.mul(marketContext.riskParameter.maintenance))

This causes existing maker positions to become misaligned with market risk parameters since _allocateMarket() will calculate new collateral requirements against these positions using the updated leverage value, potentially pushing them below minPosition or above maxPosition bounds.

The next call to rebalance() or update() will attempt to force adjust these positions through MakerStrategyLib.allocate(), as the strategy recalculates targets based on the new leverage parameters without consideration for position size transitions.

Since the vault manages pooled positions, any forced adjustments affect all users' proportional share of the vault's total assets, as position modifications are executed at the vault level rather than per individual deposit.

Recommendation:

Add a validation function to MakerStrategyLib.sol:

/// @notice Validates if positions would remain valid under new leverage
/// @param registrations Current market registrations array
/// @param marketId Market to validate
/// @param newLeverage Proposed new leverage value
/// @return isValid True if positions would remain valid under new leverage
function validatePositionsForLeverage(
    Registration[] memory registrations,
    uint256 marketId,
    UFixed6 newLeverage
) public view returns (bool isValid) {
    MarketMakerStrategyContext memory marketContext = _loadContext(registrations[marketId]);
    
    // Skip validation if no current position
    if (marketContext.currentAccountPosition.maker.isZero()) return true;
    
    // Calculate minimum required assets with new leverage
    UFixed6 newMinAssets = marketContext.riskParameter.minMargin
        .unsafeDiv(newLeverage.mul(marketContext.riskParameter.maintenance));
        
    // Check if current positions would remain valid
    UFixed6 newPositionTarget = newMinAssets
        .muldiv(newLeverage, marketContext.latestPrice.abs());
        
    return newPositionTarget.gte(marketContext.minPosition) && 
           newPositionTarget.lte(marketContext.maxPosition);
}

Modify Vault.sol updateLeverage function:

error InvalidPositionLeverageError();

function updateLeverage(uint256 marketId, UFixed6 newLeverage) external onlyOwner {
    rebalance(address(0));
    if (marketId >= totalMarkets) revert VaultMarketDoesNotExistError();
    
    // Build registrations array for validation
    Registration[] memory registrations = new Registration[](totalMarkets);
    for (uint256 i = 0; i < totalMarkets; i++) {
        registrations[i] = _registrations[i].read();
    }
    
    // Validate positions would remain valid with new leverage
    if (!MakerStrategyLib.validatePositionsForLeverage(registrations, marketId, newLeverage))
        revert InvalidPositionLeverageError();
    
    _updateMarket(marketId, UFixed6Lib.MAX, newLeverage);
}