ck - Settling and Updating Vault will fail if one market breaks

[sherlock-admin]

ck

medium

Settling and Updating Vault will fail if one market breaks

Summary

Settling and Updating Vault will fail if one market breaks

Vulnerability Detail

Updating and settling of a vault requires all the markets in use by the vault to be updated. This can be seen in the function Vault::_settleUnderlying:

    /// @notice Handles settling the vault's underlying markets
    function _settleUnderlying() private {
        for (uint256 marketId; marketId < totalMarkets; marketId++)
            _registrations[marketId].read().market.update(
                address(this),
                UFixed6Lib.MAX,
                UFixed6Lib.ZERO,
                UFixed6Lib.ZERO,
                Fixed6Lib.ZERO,
                false
            );
    }

This function is called by both the Vault::settle and Vault::update functions. If one of the markets is broken, this would lead to a revert therefore breaking these functions.

One possible way a single market would cause this revert is if the Market::update function was paused:

    function update(
        address account,
        UFixed6 newMaker,
        UFixed6 newLong,
        UFixed6 newShort,
        Fixed6 collateral,
        bool protect
    ) external nonReentrant whenNotPaused {
        Context memory context = _loadContext(account);
        _settle(context, account);
        _update(context, account, newMaker, newLong, newShort, collateral, protect);
        _saveContext(context, account);
    }

According to the contest readme, market owners should not be able to adversely affect other markets or the overall protocol. In this case a malicious owner could break the functionality of a vault.

Impact

Updating and Settling a vault are its core functions which would be broken translating into fund lockup and potential fund losses due to inability to call these functions.

Code Snippet

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

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L76-L88

Tool used

Manual Review

Recommendation

Provide a way to exclude broken markets in critical functionality such as the vault update process.

[sherlock-admin]

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

141345 commented:

x

darkart commented:

It it supposed to revert