sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

yixxas - Possible permanent DOS on market `settle()` #179

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

yixxas

medium

Possible permanent DOS on market settle()

Summary

Possible permanent DOS in market settling due to phaseId being too large.

Vulnerability Detail

Every call to settle() sync with the Chainlink registry to get the currentOracleVersion. In each call, it checks for the last updated phaseId, and syncs with the current latest Chainlink phaseId. We observe that it loops through x number of times, based on the differences in latest phaseId and last updated phaseId. It has a low risk of reaching a DOS state, if phaseId has been updated too many times from the last sync.

function sync() external returns (OracleVersion memory) {
    // Fetch latest round
    ChainlinkRound memory round = registry.getLatestRound(base, quote);

    // Revert if the round id or timestamp is 0
    if (uint64(round.roundId) == 0 || round.timestamp == 0) revert InvalidOracleRound();

    // Update phase annotation when new phase detected
    while (round.phaseId() > _latestPhaseId()) {
        _phases.push(
            Phase(
                uint128(registry.getRoundCount(base, quote, _latestPhaseId())) +
                    _phases[_phases.length - 1].startingVersion,
                uint128(registry.getStartingRoundId(base, quote, _latestPhaseId() +  1))
            )
        );
    }

    // Return packaged oracle version
    return _buildOracleVersion(round);
}

The protocol can be paused indefinitely in settle(). When it is ready to be unpaused, it may be the case that Chainlink phaseId has been updated many times since. While Chainlink updates phaseId infrequently, the odds of DOS state being reached is low. The impact however is catastrophic as protocol can never settle() since it controls the global state of the product market.

Impact

The global flywheel settle() will be DOSed and global position states cannot be updated. Furthermore, the Chainlink registry cannot be used with an already deployed oracle pair that has a large phaseId.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-oracle/contracts/ChainlinkOracle.sol#L59-L79

Tool used

Manual Review

Recommendation

Consider adding a failsafe for such instances. Decouple update of phase annotation from sync().