ck - Out of Gas error will occur if too many markets are registered to a vault

[sherlock-admin]

ck

medium

Out of Gas error will occur if too many markets are registered to a vault

Summary

Out of Gas error will occur if too many markets are registered to a vault

Vulnerability Detail

If too many markets are registered to a vault, vault functionality would be broken due to gas exhaustion. In the function Vault::_register, the number of markets registered will continue to increase with totalMarkets incremented without an upper bound.

    /// @notice Handles the registration for a new market
    /// @param market The market to register
    function _register(IMarket market) private {
        if (!IVaultFactory(address(factory())).marketFactory().instances(market)) revert VaultNotMarketError();
        if (!market.token().eq(asset)) revert VaultIncorrectAssetError();

        asset.approve(address(market));

        uint256 newMarketId = totalMarkets++;
        _registrations[newMarketId].store(Registration(market, 0, UFixed6Lib.ZERO));
        emit MarketRegistered(newMarketId, market);
    }

The issue is there is no upper bound set and this could lead to a scenario where the number of markets becomes high enough to cause gas exhaustion due to the way certain for loops are implemented.

Various functions including Vault::vlaimReward, Vault::_settleUnderlying, Vault::_manage, Vault::_loadContext, Vault::_collateral would all be impacted.

In addition, the number of loops is amplified in a function such as Vault::update which calls multiple functions which have loops in them.

    function update(
        address account,
        UFixed6 depositAssets,
        UFixed6 redeemShares,
        UFixed6 claimAssets
    ) external whenNotPaused {
        _settleUnderlying();
        Context memory context = _loadContext(account);

        _settle(context);
        _checkpoint(context);
        _update(context, account, depositAssets, redeemShares, claimAssets);
        _saveContext(context, account);
    }

Impact

The vault would be broken with multiple functionality disabled.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L140-L151

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L204-L217

Tool used

Manual Review

Recommendation

Set an upper bound on the totalMarkets value

[sherlock-admin]

2 comment(s) were left on this issue during the judging contest.

141345 commented:

l

panprog commented:

invalid because owner is trusted