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

Incomplete invalid round check in sequencer status validation

Title

Incomplete invalid round check in sequencer status validation

Summary

A missing check in the KeeperProxy contract could let the system run when it shouldn't, possibly due to network problems or issues with oracle data. This could lead to security risks.

Vulnerability Details

The _validatePrice function in KeeperProxy.sol checks if the sequencer is up but doesn't verify if the round is valid. When startedAt is 0, it means the sequencer's status hasn't been updated properly. The code currently lets the system operate even if startedAt is 0, which could be a problem.

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
(, int256 answer, uint256 startedAt, , ) = sequencerUptimeFeed.latestRoundData();
bool isSequencerUp = answer == 0;
require(isSequencerUp, "sequencer is down");
uint256 timeSinceUp = block.timestamp - startedAt;
require(timeSinceUp > GRACE_PERIOD_TIME, "Grace period not over");
}

If startedAt is 0, block.timestamp - startedAt will be a large number, which is greater than GRACE_PERIOD_TIME (3600). This means the function won't fail, even though the sequencer status might not be reliable.

Impact

Without proper checks, _validatePrice() might not fail when the sequencer feed isn't updated or is in an invalid state. This could let the system keep running when it should stop, posing operational risks.

Tools Used

Manual Review

Recommendations

Add a check to ensure startedAt isn't 0, which would indicate an invalid round. This can be done by adding the following line:

require(startedAt != 0, "Invalid sequencer round");
Updates

Lead Judging Commences

n0kto Lead Judge 8 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.