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

ChainlinkUtil checks for sequencerUptimeFeed are incorrect

Summary

ChainlinkUtil.getPrice() are not handling an edge case where sequencer price feed is in an “invalid round”.

Vulnerability Details

ChainlinkUtil.getPrice() has 2 checks that aim to validate that sequencerUptimeFeed (the sequencer on L2) is running and that the time since the update is more than the grace period.

Reference in code: link

function getPrice(
IAggregatorV3 priceFeed,
uint32 priceFeedHeartbeatSeconds,
IAggregatorV3 sequencerUptimeFeed
)
internal
view
returns (UD60x18 price)
{
...
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();
}
}
...
}

The chainlink docs say that startedAt is going to be 0 “if a round is invalid”.

Screenshot-2024-07-30-at-21-23-41.png

"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

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

Screenshot-2024-07-30-at-21-51-32.png

This means that the following check is going to pass when an “invalid round” occurs.

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

timeSinceUp will always be more than the hardcoded grace period (3600) since block.timestamp - 0 == block.timestamp

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

The checks in ChainlinkUtil.getPrice() allow for successful calls in an invalid round because code will not revert if answer == 0 and startedAt == 0 thus defeating the purpose of having a sequencerFeed check to assert 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.

Impact

The current ChainlinkUtil.getPrice() checks for the sequencer 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

Recommended Mitigation

Revert the transaction if startedAt is 0

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.