code-423n4 / 2024-07-reserve-validation

0 stars 0 forks source link

Lack of Sequencer Uptime Check for Arbitrum Deployment #204

Closed c4-bot-2 closed 1 month ago

c4-bot-2 commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/plugins/assets/OracleLib.sol#L19-L60

Vulnerability details

Summary

OracleLib::price is a function Used by asset plugins to price their collateral. However the current code logic does not make provision for the sequencer uptime feed when deployed on certain L2 chains like Arbitrum which could eventually lead to the reported prices being stale and affect the overall system.

Vulnerability Detail

It has been duly noted that the protocol plans to deploy on L2 chains like Arbitrum as stated in their docs. However, the price function in OracleLib.sol does not include checks for the sequencer uptime feed, which is crucial for deployments on Layer 2 networks like Arbitrum. On L2 networks, the sequencer plays a vital role in transaction ordering and submission. If the sequencer goes down, the reported prices may become stale without the on-chain contracts being aware of the issue.

Impact

Without sequencer uptime checks, there is a possibility of the protocol using outdated price data when the Arbitrum sequencer is down, which would lead to inaccurate pricing and possible exploitation of the system. This could eventually result in incorrect valuations or other economic vulnerabilities within the system

Proof of Concept

    function price(AggregatorV3Interface chainlinkFeed, uint48 timeout)
        internal
        view
        returns (uint192)
    {
        try chainlinkFeed.latestRoundData() returns (
            uint80 roundId,
            int256 p,
            uint256,
            uint256 updateTime,
            uint80 answeredInRound
        ) {
            if (updateTime == 0 || answeredInRound < roundId) {
                revert StalePrice();
            }

            // Downcast is safe: uint256(-) reverts on underflow; block.timestamp assumed < 2^48
            uint48 secondsSince = uint48(block.timestamp - updateTime);
            if (secondsSince > timeout + ORACLE_TIMEOUT_BUFFER) revert StalePrice();

            if (p <= 0) revert InvalidPrice();

            // {UoA/tok}
            return shiftl_toFix(uint256(p), -int8(chainlinkFeed.decimals()), FLOOR);
        } catch (bytes memory errData) {
            // Check if the aggregator was not set: if so, the chainlink feed has been deprecated
            // and a _specific_ error needs to be raised in order to avoid looking like OOG
            if (errData.length == 0) {
                if (EACAggregatorProxy(address(chainlinkFeed)).aggregator() == address(0)) {
                    revert StalePrice();
                }
                // solhint-disable-next-line reason-string
                revert();
            }

            // Otherwise, preserve the error bytes
            // solhint-disable-next-line no-inline-assembly
            assembly {
                revert(add(32, errData), mload(errData))
            }
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

In order To address this issue, implement sequencer uptime logic and checks in the price function

Assessed type

Oracle