sherlock-audit / 2023-12-flatmoney-judging

9 stars 8 forks source link

dimulski - No check if Base L2 sequencer is down in Chainlink feeds #234

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

dimulski

medium

No check if Base L2 sequencer is down in Chainlink feeds

Summary

Using Chainlink in L2 chains such as Base 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 OracleModule is used the get the the price of rETH/USD(if there was such a price feed on Base, but this is a separate issue, which once fixed still doesn't fix the issue outlined in this report) . There is no check that the sequencer is down: _getOnchainPrice()

function _getOnchainPrice() internal view returns (uint256 price, uint256 timestamp) {
        IChainlinkAggregatorV3 oracle = onchainOracle.oracleContract;
        if (address(oracle) == address(0)) revert FlatcoinErrors.ZeroAddress("oracle");

        (, int256 _price, , uint256 updatedAt, ) = oracle.latestRoundData();
        timestamp = updatedAt;
        // check Chainlink oracle price updated within `maxAge` time.
        if (block.timestamp > timestamp + onchainOracle.maxAge)
            revert FlatcoinErrors.PriceStale(FlatcoinErrors.PriceSource.OnChain);

        if (_price > 0) {
            price = uint256(_price) * (10 ** 10); // convert Chainlink oracle decimals 8 -> 18
        } else {
            // Issue with onchain oracle indicates a serious problem
            revert FlatcoinErrors.PriceInvalid(FlatcoinErrors.PriceSource.OnChain);
        }
}

The FlatMoney protocol also utilizes the Pyth oracle As per Pyth documentation: Integrators should be careful to avoid accidentally using a stale price. The Pyth SDKs guard against this failure mode by incorporating a staleness check by default. Querying the current price will fail if too much time has elapsed since the last update. The SDKs expose this failure condition in an idiomatic way: for example, the Rust SDK may return None, and our Solidity SDK may revert the transaction. Also keep in mind that if the Pyth oracle fails to get a fresh price the getPrice() function call will revert because the price returned from _getOffchainPrice() will be zero, due to this check

        uint256 priceDiff = (int256(onchainPrice) - int256(offchainPrice)).abs();
        uint256 diffPercent = (priceDiff * 1e18) / onchainPrice;
        if (diffPercent > maxDiffPercent) revert FlatcoinErrors.PriceMismatch(diffPercent);

When the OracleModule tries to fetch prices from Pyth there is the following check

if (priceData.price / int64(priceData.conf) < int32(offchainOracle.minConfidenceRatio)) {
                    invalid = true; // price confidence is too low
                }

The confidence is expected to be bigger when the volatility in the market is big. If the confidence is bigger than the one deemed reasonable by the team the invalid variable will be set to true, which is later used in order to determine which price to return in the _getPrice

    if (offchainInvalid == false) {
            // return the freshest price
            if (offchainTime >= onchainTime) {
                price = offchainPrice;
                timestamp = offchainTime;
                offchain = true;
            } else {
                price = onchainPrice;
                timestamp = onchainTime;
            }
        } else {
            price = onchainPrice;
            timestamp = onchainTime;
        }

Thus the price from the Chainlink data feed will be returned, which if the sequencer is down can be stale.

Impact

The impact depends on the usage of the OracleModule. If it is used as part of the collateral for lenders:

Users can get more profit, if they close their long positions if the price is above the actual price Users can avoid liquidations if the price is under the actual price

Code Snippet

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

Duplicate of #27

sherlock-admin commented 5 months ago

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

takarez commented:

invalid