Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Incorrect checks to verify the correct status of the sequencerUptimeFeed in ChainlinkUtil.sol contract.

nfmelendez

Summary

The ChainlinkUtil contract has sequencerUptimeFeed checks to verify if the sequencer on an L2 is running but unfortunately these checks are not implemented properly.

Vulnerability Details

The chainlink documentation states that the sequencerUptimeFeed may return a 0 value for startedAt if it is queried during an "invalid round."

startedAt: This timestamp indicates when the sequencer changed status. This timestamp returns 0 if a round is invalid. When the sequencer comes back up after an outage, wait for the GRACE_PERIOD_TIME to pass before accepting answers from the data feed. Subtract startedAt from block.timestamp and revert the request if the result is less than the GRACE_PERIOD_TIME.
​
If the sequencer is up and the GRACE_PERIOD_TIME has passed, the function retrieves the latest answer from the data feed using the dataFeed object.
​

An invalid round is described to mean there was a problem updating the sequencer's status, possibly due to network issues or problems with data from oracles, and is shown by a startedAt time of 0 and answer is 0.

This makes check below in ChainlinkUtil::getPrice to be useless if its called in an invalid round:

bool isSequencerUp = answer == 0;
if (!isSequencerUp) {
revert Errors.OracleSequencerUptimeFeedIsDown(address(sequencerUptimeFeed));
}
​
uint256 timeSinceUp = block.timestamp - startedAt;
if (timeSinceUp <= Constants.SEQUENCER_GRACE_PERIOD_TIME) {// @audit started=0 for invalid round so timeSinceUp will always be > SEQUENCER_GRACE_PERIOD_TIME
revert Errors.GracePeriodNotOver();
}

Chainlink secuencer feed when a round start set startedAt = 0 and answer = 0. Later, both the answer and the time it was updated (updatedAt) are set at the same time after getting enough data from oracles, making sure that answer only changes from 0 when there’s a confirmed update different from the start time, So for an invalid round startedAt = 0 and answer = 0 and note that answer=0 means that the secuencer is up but startedAt=0 is saying was an invalid round so the answer can't be trusted. Making sure startedAt isn’t 0 is crucial for keeping the system secure and properly informed about the sequencer’s status.

In the Case of Zaros protocol in an invalid round startedAt = 0 and answer = 0:

  1. Won't revert with OracleSequencerUptimeFeedIsDown because answer is 0

  2. Won't revert GracePeriodNotOver because timeSinceUp = block.timestamp - 0 -> timeSinceUp == block.timestamp -> timeSinceUp will always be greater than SEQUENCER_GRACE_PERIOD_TIME

  3. But the secuencer could be down

Code lines

Impact

Missing checks to confirm the correct status of the SequencerUptimeFeed in ChainlinkUtil::getPrice will cause getPrice() to not revert when the SequencerUptimeFeed is down in an invalid round.

Tools Used

  • VS Code

Recommendations

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

// @audit new check here
bool isInvalidRound = startedAt == 0;
if (isInvalidRound) {
revert Errors.OracleSequencerInvalidRound(address(sequencerUptimeFeed));
}
​
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();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months 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.