Cheerful Taffy Dolphin - Array Length Mismatch in Vault's _manage() Function Could Lead to Fund Loss via Invalid Position Management

[sherlock-admin3]

Cheerful Taffy Dolphin

Medium

Array Length Mismatch in Vault's _manage() Function Could Lead to Fund Loss via Invalid Position Management

Summary

A critical vulnerability has been identified in the _manage function's memory handling. This function serves as the core mechanism for managing market positions and collateral allocation across registered markets. The vulnerability stems from unsafe array access patterns between the context's registrations and strategy-generated targets, potentially compromising the vault's financial operations and position management.

The vulnerability centers on array length management between context.registrations and the targets array returned by the virtual _strategy function. In the current implementation:

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

function _manage(Context memory context, UFixed6 deposit, UFixed6 withdrawal, bool shouldRebalance) private {
    Target[] memory targets = _strategy(context, deposit, withdrawal, _ineligible(context, deposit, withdrawal));
    
    for (uint256 marketId; marketId < context.registrations.length; marketId++)
        if (targets[marketId].collateral.lt(Fixed6Lib.ZERO))
            _retarget(context.registrations[marketId], targets[marketId], shouldRebalance);

The core issue lies in the array access pattern where the loop iterates using context.registrations.length but accesses targets[marketId] without any length verification. Since _strategy is a virtual function, its implementation could return an array of any length, creating a critical mismatch.

Impact

When targets.length < context.registrations.length, this leads to out-of-bounds access and potential contract reversion. Conversely, if targets.length > context.registrations.length, some target positions remain unprocessed, leading to incomplete strategy execution.

The financial implications are severe – mismatched array lengths can result in incorrect position management, unbalanced collateral distribution, and potential fund loss through improper allocations. The system's risk management capabilities are compromised as market exposure becomes misaligned with intended strategy.

Fix

Implement strict length validation:

function _manage(Context memory context, UFixed6 deposit, UFixed6 withdrawal, bool shouldRebalance) private {
    Target[] memory targets = _strategy(context, deposit, withdrawal, _ineligible(context, deposit, withdrawal));
    require(targets.length == context.registrations.length, "Length mismatch");
    // ... existing logic
}

Alternatively, a more flexible approach using dynamic length handling:

function _manage(Context memory context, UFixed6 deposit, UFixed6 withdrawal, bool shouldRebalance) private {
    Target[] memory targets = _strategy(context, deposit, withdrawal, _ineligible(context, deposit, withdrawal));
    require(targets.length <= context.registrations.length, "Too many targets");
    
    for (uint256 marketId; marketId < targets.length; marketId++) {
        if (targets[marketId].collateral.lt(Fixed6Lib.ZERO))
            _retarget(context.registrations[marketId], targets[marketId], shouldRebalance);
    }
}