Topmark - sequencerDown would Return True even During Grace Period

[sherlock-admin]

Topmark

medium

sequencerDown would Return True even During Grace Period

Summary

sequencerDown would Return True even During Grace Period in the Registry contract

Vulnerability Detail

function _isSequencerDown(address creditor) internal view returns (bool success, bool sequencerDown) {
        // This guarantees that no stale oracles are consumed when the sequencer is down,
        // and that Account owners have time (the grace period) to bring their Account back in a healthy state.
        try IChainLinkData(sequencerUptimeOracle).latestRoundData() returns (
            uint80, int256 answer, uint256 startedAt, uint256, uint80
        ) {
            success = true;
 >>>           if (answer == 1 || block.timestamp - startedAt < riskParams[creditor].gracePeriod) {
                sequencerDown = true;
            }
        } catch { }
    }

As noted in the code above the _isSequencerDown(...) Function from the pointer shows how validation is done before sequencerDown returns true it can be noted that in a situation the time that is been used i.e (block.timestamp - startAt) is less that grace period, true should be return this is totally wrong the sequencer down should only return true after the current time has crossed over the grace period. i.e grace period has been exhausted.

Impact

sequencerDown would Return True even During Grace Period in the Registry contract

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/accounts-v2/src/Registry.sol#L174

Tool used

Manual Review

Recommendation

As adjusted below greater than condition should be used Instead of less than to ensure Sequencer is not down during grace period.

function _isSequencerDown(address creditor) internal view returns (bool success, bool sequencerDown) {
        // This guarantees that no stale oracles are consumed when the sequencer is down,
        // and that Account owners have time (the grace period) to bring their Account back in a healthy state.
        try IChainLinkData(sequencerUptimeOracle).latestRoundData() returns (
            uint80, int256 answer, uint256 startedAt, uint256, uint80
        ) {
            success = true;
 ---           if (answer == 1 || block.timestamp - startedAt < riskParams[creditor].gracePeriod) {
 +++          if (answer == 1 || block.timestamp - startedAt > riskParams[creditor].gracePeriod) {
                sequencerDown = true;
            }
        } catch { }
    }

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid

[nevillehuang]

Invalid, logic is correct to prevent stale oracle prices during sequencer down time. The check ensures that grace period must have passed once oracle resumes that is calculated as the time difference between current timestamp and last timestamp oracle resume should. See here for more information.