Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

KeeperProxy Fails to Properly Validate Sequencer Uptime in Invalid Rounds

wildtiger198

Summary

The KeeperProxy contract includes sequencerUptimeFeed checks to verify whether the sequencer on Arbitrum is operational. However, these checks are not implemented correctly. Since the protocol already performs some validation of the sequencerUptimeFeed status, it should ensure all necessary checks are in place to avoid security risks.

Vulnerability Details

According to the chainlink documentation, the sequencerUptimeFeed can return a startedAt value of 0 when queried 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.

An "invalid round" occurs when there is an issue updating the sequencer’s status, possibly due to network disruptions or insufficient oracle data. This is indicated by both startedAt = 0 and answer = 0. Further clarification was provided by an official Chainlink engineer in the chainlink public discord.

Hello, @EricTee 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.

A key takeaway from Chainlink’s documentation and developer statement is:

Ensuring that startedAt is not 0 is crucial for maintaining security and accurately determining the sequencer's status.

The KeeperProxy::_validatePrice function contains an L2 sequencer check:

// L2 Sequencer check
(
/*uint80 roundID*/,
int256 answer,
uint256 startedAt,
/*uint256 updatedAt*/,
/*uint80 answeredInRound*/
) = AggregatorV2V3Interface(sequencerUptimeFeed).latestRoundData();
bool isSequencerUp = answer == 0;
require(isSequencerUp, "sequencer is down");
// Make sure the grace period has passed after the sequencer is back up.
uint256 timeSinceUp = block.timestamp - startedAt;
require(timeSinceUp > GRACE_PERIOD_TIME, "Grace period is not over");

This check is flawed when executed during an "invalid round":

  • If startedAt = 0, the arithmetic operation block.timestamp - startedAt results in a value much greater than GRACE_PERIOD_TIME.

  • For example, if block.timestamp = 1719739032, then 1719739032 - 0 = 1719739032, which is far greater than the required GRACE_PERIOD_TIME(hardcoded as 3600).

  • As a result, the require(timeSinceUp > GRACE_PERIOD_TIME, "Grace period is not over"); condition never reverts, allowing function execution to proceed even if the sequencer uptime feed is in an invalid state.

This creates a security risk where validatePrice() may incorrectly determine that the sequencer is operational when, in reality, the data is not properly updated.

Impact

The failure to properly validate sequencerUptimeFeed status allows _validatePrice() to succeed even when the sequencer uptime feed is in an invalid round.

Tools Used

Manual Review

Recommendations

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

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
// L2 Sequencer check
(
/*uint80 roundID*/,
int256 answer,
uint256 startedAt,
/*uint256 updatedAt*/,
/*uint80 answeredInRound*/
) = AggregatorV2V3Interface(sequencerUptimeFeed).latestRoundData();
bool isSequencerUp = answer == 0;
require(isSequencerUp, "sequencer is down");
+ require(startedAt != 0, "Invalid sequencer round");
// Make sure the grace period has passed after the sequencer is back up.
uint256 timeSinceUp = block.timestamp - startedAt;
require(timeSinceUp > GRACE_PERIOD_TIME, "Grace period is not over");
​
address market = IPerpetualVault(perpVault).market();
IVaultReader reader = IPerpetualVault(perpVault).vaultReader();
MarketProps memory marketData = reader.getMarket(market);
_check(marketData.indexToken, prices.indexTokenPrice.min);
_check(marketData.indexToken, prices.indexTokenPrice.max);
_check(marketData.longToken, prices.indexTokenPrice.min);
_check(marketData.longToken, prices.indexTokenPrice.max);
_check(marketData.shortToken, prices.shortTokenPrice.min);
_check(marketData.shortToken, prices.shortTokenPrice.max);
}
Updates

Lead Judging Commences

n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_sequencerUptimeFeed_startedAt_0_no_roundId

startedAt is only 0 when contract is not initialized on Arbitrum, but it is already initialized on Arbitrum. startedAt is sufficient for the protocol, it does not need roundID. Current documentation of Chainlink does not have this sentence: “This timestamp returns `0` if a round is invalid.“

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.