sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

0xblackskull - Chainlink's `latestRoundData()` might return stale or incorrect results #236

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 8 months ago

0xblackskull

medium

Chainlink's latestRoundData() might return stale or incorrect results

Summary

Chainlink's latestRoundData() is used but there is no check if the return value indicates stale data. This could lead to stale prices according to the Chainlink documentation: https://docs.chain.link/docs/historical-price-data/#historical-rounds

Vulnerability Detail

The OracleModule::_getOnchainPrice() and KeeperFee::getKeeperFee() uses Chainlink's latestRoundData() to get the latest price. However, there is no check if the return value indicates stale data.

According to Chainlink's documentation, this function does not error if no answer has been reached but returns 0 or outdated round data. The external Chainlink oracle, which provides index price information to the system, introduces risk inherent to any dependency on third-party data sources.

Impact

The OracleModule::_getOnchainPrice() and KeeperFee::getKeeperFee() could return stale price data for the underlying asset.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L145

function _getOnchainPrice() internal view returns (uint256 price, uint256 timestamp) {
        IChainlinkAggregatorV3 oracle = onchainOracle.oracleContract;
        if (address(oracle) == address(0)) revert FlatcoinErrors.ZeroAddress("oracle");
        // @audit [M-1] oracle stale price
        (, int256 _price, , uint256 updatedAt, ) = oracle.latestRoundData();
       .......
    }

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/misc/KeeperFee.sol#L84

function getKeeperFee() public view returns (uint256 keeperFeeCollateral) {
        uint256 ethPrice18;
        {
            (, int256 ethPrice, , uint256 ethPriceupdatedAt, ) = _ethOracle.latestRoundData();
            if (block.timestamp >= ethPriceupdatedAt + _STALENESS_PERIOD) revert FlatcoinErrors.ETHPriceStale();
            if (ethPrice <= 0) revert FlatcoinErrors.ETHPriceInvalid();
            ethPrice18 = uint256(ethPrice) * 1e10; // from 8 decimals to 18
        }
        .........

Tool used

Manual Review

Recommendation

function _getOnchainPrice() internal view returns (uint256 price, uint256 timestamp) {
    ............
    (uint80 roundId, int256 _price, , uint256 updatedAt, uint80 answeredInRound)) = oracle.latestRoundData();
        require(answeredInRound >= roundId, "Price stale");
       .......
    }
function getKeeperFee() public view returns (uint256 keeperFeeCollateral) {
        uint256 ethPrice18;
        {
            (uint80 roundId, int256 ethPrice, , uint256 ethPriceupdatedAt, uint80 answeredInRound) = _ethOracle.latestRoundData();
           require(answeredInRound >= roundId, "Price stale");
        }
        .........

Duplicate of #3

sherlock-admin commented 7 months ago

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

takarez commented:

invalid