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

Inadequate checks to confirm the correct status of the sequecnce/sequencerUptimeFeed

Summary

The startedAt value returned by sequencerUptimeFeed.latestRoundData() should be checked against zero as stated in the Chainlink documentation at the bottom of the code example.

Vulnerability Details

The chainlink docs (very end of article, below code example) 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.

This was also confirmed by chainlink team:

This makes the implemented check below to be useless if its called in an invalid round (meaning startedAt == 0), as block.timestamp - startedAt will always be > Constants.SEQUENCER_GRACE_PERIOD_TIME:

File: src/external/chainlink/ChainlinkUtil.sol
41: if (address(sequencerUptimeFeed) != address(0)) {
42: try sequencerUptimeFeed.latestRoundData() returns (
43: uint80, int256 answer, uint256 startedAt, uint256, uint80
44: ) {
45: bool isSequencerUp = answer == 0;
46: if (!isSequencerUp) {
47: revert Errors.OracleSequencerUptimeFeedIsDown(address(sequencerUptimeFeed));
48: }
49:
50: uint256 timeSinceUp = block.timestamp - startedAt;
51:❌ if (timeSinceUp <= Constants.SEQUENCER_GRACE_PERIOD_TIME) {
52: revert Errors.GracePeriodNotOver();
53: }
54: } catch {
55: revert Errors.InvalidSequencerUptimeFeedReturn();
56: }
57: }

Impact

inadequate checks to confirm the correct status of the sequencerUptimeFeed in ChainlinkUtil::getPrice() contract will cause getPrice() to not revert even when the sequencer uptime feed is not updated or is called in an invalid round.

Tools Used

Manual review

Recommendations

Add a check for startedAt == 0, this is an example but you might prefer to process this scenario differently:

--- a/src/external/chainlink/ChainlinkUtil.sol
+++ b/src/external/chainlink/ChainlinkUtil.sol
@@ -51,6 +51,9 @@ library ChainlinkUtil {
if (timeSinceUp <= Constants.SEQUENCER_GRACE_PERIOD_TIME) {
revert Errors.GracePeriodNotOver();
}
+ if(startedAt == 0) {
+ revert Errors.InvalidRound();
+ }
} catch {
revert Errors.InvalidSequencerUptimeFeedReturn();
}
Updates

Lead Judging Commences

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