Zaros Part 1

Summary

ChainlinkUtil.getPrice() fails to properly handle cases where the sequencer uptime feed returns an "invalid round" (indicated by startedAt being 0), allowing price fetching when the sequencer status is uncertain or incorrectly reported.

Vulnerability Details

The PriceFeed 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".

Please bear in mind 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 explanations can be seen as given by an official chainlink engineer as seen here in the chainlink public discord:

https://discord.com/channels/592041321326182401/605768708266131456/1213847312141525002

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

as startedAt will be 0, the arithmetic operation block.timestamp - startedAt will result in a value greater than SEQUENCER_GRACE_PERIOD_TIME (which is hardcoded to be 3600) as seen here. That is, block.timestamp = 1826745042, so 1826745042 - 0 = 1826745042 which is bigger than 3600. The code won't revert.

Consider 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 the docs say that if answer = 0, sequencer is up BUT 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.

With 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 sucessful calls in an invalid round because reverts doesn't happen if answer == 0 and startedAt == 0; thus defeating the purpose of having a sequencerFeed check to ascertain 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).

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

Impact

Inadequate checks to confirm the correct status of the sequencer/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

• Chainlink docs

Recommended Mitigation Steps

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