DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: medium
Valid

Inadequate checks to confirm the correct status of the sequecnce/sequecncerUptimeFeed in ChainLink.getPrice() contract

Summary

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

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/external/chainlink/ChainlinkUtil.sol#L43

Vulnerability Details

if (address(sequencerUptimeFeed) != address(0)) {
try sequencerUptimeFeed.latestRoundData() returns (
uint80, int256 answer, uint256 startedAt, uint256, uint80
) {
@> bool isSequencerUp = answer == 0;
if (!isSequencerUp) {
revert Errors.OracleSequencerUptimeFeedIsDown(address(sequencerUptimeFeed));
}
uint256 timeSinceUp = block.timestamp - startedAt;
if (timeSinceUp <= Constants.SEQUENCER_GRACE_PERIOD_TIME) {
revert Errors.GracePeriodNotOver();
}
} catch {
revert Errors.InvalidSequencerUptimeFeedReturn();
}
}

as startedAt will be 0, the arithmetic operation block.timestamp - startedAt will result in a value greater than GRACE_PERIOD_TIME (which is hardcoded to be 3600) i.e block.timestamp = 1719739032, so 1719739032 - 0 = 1719739032 which is bigger than 3600. The code won't revert.

Imagine a case where a round starts, at the beginning startedAt is recorded to be 0, and answer, the initial status is set to be 0. Note that docs say that if answer = 0, sequencer is up, if equals to 1, sequencer is down. But in this case here, answer and startedAt can be 0 initially, till after all data is gotten from oracles and update is confirmed then the values are reset to the correct values that show the correct status of the sequencer.

From these explanations and information, it can be seen that startedAt value is a second value that should be used in the check for if a sequencer is down/up or correctly updated. The checks in ChainlinkUtil.getPrice() will allow for sucessfull calls in an invalid round because reverts dont happen if answer == 0 and startedAt == 0 thus defeating the purpose of having a sequencerFeed check to assertain the status of the sequencerFeed on L2 i.e if it is up/down/active or if its status is actually confirmed to be either.

Impact

inadequate checks to confirm the correct status of the sequecncer/sequecncerUptimeFeed in ChainlinkUtil.getPrice() contract will cause getPrice() to not revert inside the try-catch even when the sequcncer uptime feed is not updated or is called in an invalid round.

Tools Used

Manual Review

Recommendations

add a check that reverts if startedAt is returned as 0.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Inadequate implementation of sequencer check

Support

FAQs

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