sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

seeques - `sync()` doesn't check whether Arbitrum sequencer is down #191

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

seeques

medium

sync() doesn't check whether Arbitrum sequencer is down

Summary

Downtimes happen more often than not on L2s so it is important to validate if Arbitrum sequencer is operational before any state-changing operation occurs. This is to ensure that the prices are fresh and not stale so that users will not be harmed while actively managing their positions.

Vulnerability Detail

In ChainlinkFeedOracle's and ChainlinkOracle's sync() function there is no check if the sequencer is down. E.g. ChainlinkOracle code:

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);
    }

Impact

Users with assumptions that the prices are not stale could be potentially harmed while actively managing their positions.

Code Snippet

Tool used

Manual Review

Recommendation

Follow the Chainlink's example code: https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code

Duplicate of #13