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

Inadequate checks to confirm the correct status of the sequencerUptimeFeed in ChainlinkUtil.getPrice()

Lines of code

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

Impact

Inadequate checks to confirm the correct status of the sequecncerUptimeFeed in ChainlinkUtil.getPrice() contract will cause getPrice() to not revert even when the sequecncerUptimeFeed is not updated or is called in an invalid round.

Vulnerability details

When getting the price for a market through ChainlinkUtil.getPrice, first it is asserted that the sequencer 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":

• 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.

Please note that 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. Further explanation can be seen as given by an official chainlink engineer as seen here in the chainlink public discord

An "invalid round" means 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. Normally, when a round starts,
startedAt is recorded, and the initial status (answer) is set to 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. This process
helps avoid mistakes in judging if the sequencer is available, which could cause security issues.
Making sure startedAt isn't 0 is crucial for keeping the system secure and properly informed about the
sequencer's status.

This makes the implemented check below in the ChainlinkUtil.getPrice() to be useless if it is called in an invalid round:

uint256 timeSinceUp = block.timestamp - startedAt;
if (timeSinceUp <= Constants.SEQUENCER_GRACE_PERIOD_TIME) {
revert Errors.GracePeriodNotOver();
}

as startedAt will be 0, block.timestamp - startedAt will result in a value greater than GRACE_PERIOD_TIME (which is hardcoded to be 3600) and 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.

Based on the explenations aboge, the startedAt value should be used in the check for if a sequencer is down/up or correctly updated.

Recommended Mitigation Steps

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

uint256 timeSinceUp = block.timestamp - startedAt;
if (timeSinceUp <= Constants.SEQUENCER_GRACE_PERIOD_TIME) {
revert Errors.GracePeriodNotOver();
}
+ if(startedAt == 0){
+ revert;
+ }
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.