sherlock-audit / 2024-02-smilee-finance-judging

2 stars 1 forks source link

smbv-1919 - No check if Arbitrum L2 sequencer is down in ChainlinkPriceOracle #141

Closed sherlock-admin3 closed 8 months ago

sherlock-admin3 commented 9 months ago

smbv-1919

medium

No check if Arbitrum L2 sequencer is down in ChainlinkPriceOracle

Summary

-Using Chainlink Oracle in L2 chains such as Arbitrum requires to check if the sequencer is down to avoid prices from looking like they are fresh although they are not.

-The bug could be leveraged by malicious actors to take advantage of the sequencer downtime.

Vulnerability Detail

The getTokenPrice() is used the get the the price of a token. There is no check that the sequencer is down. https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/chainlink/ChainlinkPriceOracle.sol#L90 https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/chainlink/ChainlinkPriceOracle.sol#L110

function getTokenPrice(address token) public view returns (uint256) {
        if (token == address(0)) {
            revert AddressZero();
        }

        address priceFeed = feeds[token].get();
        if (priceFeed == address(0)) {
            revert TokenNotSupported();
        }

        OracleValue memory price = _getFeedValue(priceFeed);

        // Protect against stale feeds:
        if (block.timestamp - price.lastUpdate > getPriceFeedMaxDelay(token)) {
            revert PriceTooOld();
        }

        return price.value;
    }

    function _getFeedValue(address priceFeedAddr) internal view returns (OracleValue memory datum) {
        AggregatorV3Interface priceFeed = AggregatorV3Interface(priceFeedAddr);
        /*
            latestRoundData SHOULD raise "No data present"
            if they do not have data to report, instead of returning unset values
            which could be misinterpreted as actual reported values.
        */
        (, int256 answer, , uint256 updatedAt, ) = priceFeed.latestRoundData();

        if (answer < 0) {
            revert PriceNegative();
        }

        datum.value = AmountsMath.wrapDecimals(uint256(answer), priceFeed.decimals());
        datum.lastUpdate = updatedAt;
    }

Impact

-If the Arbitrum sequencer goes down, the protocol will allow users to continue to operate at the previous (stale) rates.

Code Snippet

https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/chainlink/ChainlinkPriceOracle.sol#L90 https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/chainlink/ChainlinkPriceOracle.sol#L110

Tool used

Manual Review

Recommendation

-It is recommended to follow the code example of Chainlink: https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code

sherlock-admin4 commented 8 months ago

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

panprog commented:

invalid, sequencer issues are invalid in sherlock, besides the underlying DEX price will prevent the usage of outdated oracle price

tsvetanovv commented:

According to Smilee Readme and Sherlock documentation this issue type is invalid

takarez commented:

invalid