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

2 stars 1 forks source link

calpaliu - Chainlink's `latestRoundData` might return stale or incorrect results #2

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

calpaliu

medium

Chainlink's latestRoundData might return stale or incorrect results

Summary

The provided Solidity code snippet retrieves data from an oracle contract to obtain the latest price feed information. However, it lacks explicit error handling for scenarios where the data is stale, which could lead to inaccurate results. Additionally, it does not validate the roundID, leaving the code vulnerable to potential manipulation or returning stale data from the oracle.

Vulnerability Detail

The code currently verifies the freshness of data solely based on the timestamp (updatedAt). However, without validating the roundID, there's a risk of returning stale data or being misled by outdated information. Stale data could occur if the contract has updated its timestamp but not its roundID, leading to erroneous interpretations of the data's freshness.

Impact

The absence of explicit error handling for stale data and the lack of validation for the roundID increase the likelihood of returning outdated or inaccurate information. This vulnerability could result in incorrect decisions or actions based on unreliable data, potentially leading to financial losses, system malfunctions, or security vulnerabilities.

Code Snippet

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;
}

Tool Used

Manual Review

Recommendation

(
    uint80 roundID,
    int256 answer,
    uint256 startedAt,
    uint256 updatedAt,
    uint80 answeredInRound
) = priceFeed.latestRoundData();

require(
    answeredInRound >= roundID,
    “ChainlinkOracle::getLatestAnswer - Stale Data”
);

Add a check to ensure that the retrieved roundID matches the answeredInRound value returned by the oracle contract. This validation ensures that the data corresponds to the latest round, mitigating the risk of returning stale data or being misled by outdated information.

sherlock-admin3 commented 7 months ago

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

tsvetanovv commented:

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

takarez commented:

invalid; this is invalid according to sherlock rules VII. Num 17