DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Incorrect checks to verify the correct status of the sequencerUptimeFeed in KeeperProxy.sol contract.

Summary

The KeeperProxy contract has sequencerUptimeFeed checks to verify if the sequencer on an L2 is running but unfortunately these checks are not implemented properly.

Vulnerability Details

The chainlink documentation states that the sequencerUptimeFeed may return a 0 value for startedAt if it is 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.
If the sequencer is up and the GRACE_PERIOD_TIME has passed, the function retrieves the latest answer from the data feed using the dataFeed object.

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.

This makes check below in KeeperProxy::_validatePrice to be useless if its called in an invalid round:

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");
uint256 timeSinceUp = block.timestamp - startedAt; // <@audit
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);
}

Chainlink secuencer feed when a round start set startedAt = 0 and answer = 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, So for an invalid round startedAt = 0 and answer = 0 and note that answer=0 means that the secuencer is up but startedAt=0 is saying was an invalid round so the answer can't be trusted. Making sure startedAt isn’t 0 is crucial for keeping the system secure and properly informed about the sequencer’s status.

In the Case of Gamma protocol in an invalid round startedAt = 0 and answer = 0:

  1. Won't revert with "secuencer is down because answer is 0

  2. Won't revert with "Grace period is not over" because timeSinceUp = block.timestamp - 0 -> timeSinceUp == block.timestamp -> timeSinceUp will always be greater than GRACE_PERIOD_TIME

  3. But the secuencer could be down

https://github.com/CodeHawks-Contests/2025-02-gamma/blob/main/contracts/KeeperProxy.sol#L164-L168

Impact

  • Missing checks to confirm the correct status of the SequencerUptimeFeed in Keeper::_validatePrice will cause run() and runNextAction() to not revert when the SequencerUptimeFeed is down in an invalid round.

Tools Used

  • VS Code

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 isValidRound = startedAt != 0; //<@ audit
require(isValidRound, "invalid round");// <@ audit
bool isSequencerUp = answer == 0;
require(isSequencerUp, "sequencer is down");
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 9 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.

Give us feedback!