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

Insufficient validation of the sequencerUptimeFeed in `ChainlinkUtil::getPrice()`

Summary

Insufficient validation of the sequencerUptimeFeed in ChainlinkUtil::getPrice()

Vulnerability Details

This is how sequencerUptimeFeedis validated:

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

However, it is not correctly validated. The startedAtcan be 0 during an invalid round but that is not validated in the code above. Check the [Chainink Docs](https://docs.chain.link/data-feeds/l2-sequencer-feeds) and see for yourself. This is what you can see there:

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.

This makes the check below insufficient as timeSinceUp will equal block.timestampsince startedAt will be 0 making the check always pass.

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

You can also check what a Chainlink developer said in their public discord ([Message Link]https://discord.com/channels/592041321326182401, you will have to join their server to see)

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.

Impact

Insufficient validation of the sequencerUptimeFeed in ChainlinkUtil::getPrice()causing potentially wrong data to be used by the protocol during an invalid round

Tools Used

Manual Review

Recommendations

Implement a check for the startedAt value

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.