Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Inadequate checks to confirm the correct status of the sequencerUptimeFeed

0xdice91

Summary

The ChainlinkUtil contract has sequencerUptimeFeed checks in place to assert if the sequencer on the L2 is running but these checks are not implemented correctly. According to the chainlink docs the sequencerUptimeFeed can return a 0 value for startedAt if it is called during an "invalid round".

Vulnerability Details

Consider a scenario where a round begins with `startedAt` recorded as 0 and the answer is initially set to 0. According to the documentation, an `answer` of 0 indicates the sequencer is up, while an `answer` of 1 means the sequencer is down. However, in this situation, both `answer` and `startedAt` can initially be `0` until all data is received from oracles and the update is confirmed. After this, the values are adjusted to reflect the accurate status of the sequencer.

in such a scenario the checks in _validatePrice() will not sufficiently check against an invalid sequencer as `answer` will be zero and so will `startedAt` passing the second check as `block.timestamp` minus o will still be block.timestamp which is greater than GRACE_PERIOD_TIME.

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
// L2 Sequencer check
(
/*uint80 roundID*/,
int256 answer,
uint256 startedAt,
/*uint256 updatedAt*/,
/*uint80 answeredInRound*/
) = AggregatorV2V3Interface(sequencerUptimeFeed).latestRoundData();
bool isSequencerUp = answer == 0;
require(isSequencerUp, "sequencer is down");
// Make sure the grace period has passed after the sequencer is back up.
uint256 timeSinceUp = block.timestamp - startedAt;
require(timeSinceUp > GRACE_PERIOD_TIME, "Grace period is not over");
​
address market = IPerpetualVault(perpVault).market();
IVaultReader reader = IPerpetualVault(perpVault).vaultReader();
MarketProps memory marketData = reader.getMarket(market);
_check(marketData.indexToken, prices.indexTokenPrice.min);
_check(marketData.indexToken, prices.indexTokenPrice.max);
_check(marketData.longToken, prices.indexTokenPrice.min);
_check(marketData.longToken, prices.indexTokenPrice.max);
_check(marketData.shortToken, prices.shortTokenPrice.min);
_check(marketData.shortToken, prices.shortTokenPrice.max);
}

It has been established that startedAt is another important value that must be verified to ensure an active sequencer, with the current implementation of getPrice() an invalid can still be passed active thus defeating the purpose of a sequencerFeed check

Impact

This will lead to getPrice() not reverting in an invalid round, which goes against the intention of the developers.

Tools Used

Manual Review

Recommendations

A check should be added that reverts if `startedAt` is returned as 0.

Updates

Lead Judging Commences

n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_sequencerUptimeFeed_startedAt_0_no_roundId

startedAt is only 0 when contract is not initialized on Arbitrum, but it is already initialized on Arbitrum. startedAt is sufficient for the protocol, it does not need roundID. Current documentation of Chainlink does not have this sentence: “This timestamp returns `0` if a round is invalid.“

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.