ADD-02 Unmitigated

[c4-bot-5]

Lines of code

https://github.com/revert-finance/lend/blob/audit/src/V3Oracle.sol?plain=1#L360

Vulnerability details

Lines of code

https://github.com/revert-finance/lend/blob/audit/src/V3Oracle.sol?plain=1#L360

Vulnerability details

C4 issue

ADD-02: Missing L2 sequencer checks for Chainlink oracle

Comments

Revert plans to deploy on Arbitrum. Because of this, Revert needs to add a Chainlink sequencer check when retrieving the Chainlink price. This check ensures that the Chainlink price retrieved is active and healthy. Unfortunately, V3Oracle._getChainlinkPriceX96() does not check if the Chainlink sequencer is down, leading to the possibility that Revert will use outdated Chainlink prices.

Mitigation

PR #27

Revert mitigates this issue by adding a sequencer uptime safety check to the V3Oracle contract. This feature includes the following added changes:

• Add a sequencerUptimeFeed state variable which represents the address to check for the uptime feed.
• Add a SEQUENCER_GRACE_PERIOD_TIME state variable which represents how long Revert is willing to wait before it considers the sequencer active and healthy.
• Add a setSequencerUptimeFeed() function that allows the owner to set the uptime feed address.
• Add several guard checks in _getChainlinkPriceX96() to ensure that both the sequencer is up and that enough of a grace period has passed before Revert accepts the latest Chainlink prices.

The last point is a near copy of the Chainlink example code found here.

New issue

While checking to see if the sequencer is up, Revert will revert (no pun intended) the transaction if the sequencer is up.

Code snippet

// Answer == 0: Sequencer is up
// Answer == 1: Sequencer is down
if (sequencerAnswer == 0) {
  revert SequencerDown();
}

Impact

If the sequencer uptime feed address is set in the contract, the function will always revert when the sequencer is up.

Proof of Concept

By reviewing the code in the code snippet as well as referencing the Chainlink documentation, it is noted that a sequencerAnswer of 0 means that the sequencer is up. If the value is 1, then the sequencer is down. However, the if statement will revert if sequencerAnswer == 0. This means any time the sequencer is up, the function will revert.

Tools used

Manual Review

Recommended additional mitigation

Update the conditional statement such that:

if (sequencerAnswer == 1) {
  revert SequencerDown();
}

Conclusion

Mitigation error

Assessed type

Library

[c4-judge]

jhsagd76 marked the issue as new finding

[c4-judge]

jhsagd76 marked the issue as satisfactory

[c4-judge]

jhsagd76 marked the issue as duplicate of #5