Oxhunter526 - Chainlink's `latestRoundData` return stale or incorrect result

[sherlock-admin2]

Oxhunter526

medium

Chainlink's latestRoundData return stale or incorrect result

Summary

The function getPriceInEth in the provided code lacks a comprehensive check for stale or outdated data returned by the Chainlink Oracle's latestRoundData function. This omission could lead to the usage of inaccurate or obsolete price information for tokens.

Vulnerability Detail

The getPriceInEth function is designed to retrieve the price of a token in terms of Ethereum (ETH) from the Chainlink Oracle network. While the code includes checks for the round ID, price, and timestamp, it does not adequately address the possibility of receiving stale or outdated data from the Chainlink Oracle.

    function getPriceInEth(address token) external returns (uint256) {//TODO: We're getting price from chainLink Oracle network
        ChainlinkInfo memory chainlinkOracle = _getChainlinkInfo(token);

        // Partial return values are intentionally ignored. This call provides the most efficient way to get the data.
        // slither-disable-next-line unused-return
        (uint80 roundId, int256 price,, uint256 updatedAt,) = chainlinkOracle.oracle.latestRoundData();
        uint256 timestamp = block.timestamp;
        uint256 oracleStoredTimeout = uint256(chainlinkOracle.pricingTimeout);
        uint256 tokenPricingTimeout = oracleStoredTimeout == 0 ? DEFAULT_PRICING_TIMEOUT : oracleStoredTimeout;
        if (
            roundId == 0 || price <= 0 || updatedAt == 0 || updatedAt > timestamp//TODO: timestamp to check if chainLink price feed is up to date
                || updatedAt < timestamp - tokenPricingTimeout
        ) revert InvalidDataReturned();

        uint256 decimals = chainlinkOracle.decimals;
        // Checked to be > 0 above.
        uint256 priceUint = uint256(price);
        // Chainlink feeds have certain decimal precisions, does not neccessarily conform to underlying asset.
        uint256 normalizedPrice = decimals == 18 ? priceUint : priceUint * 10 ** (18 - decimals);

        return _denominationPricing(chainlinkOracle.denomination, normalizedPrice, token);
    }

Specifically, the following factors contribute to this vulnerability:

1. The code validates the freshness of the data primarily based on the comparison of the updatedAt timestamp with the current block's timestamp. If the updatedAt timestamp is earlier than a certain threshold (defined by tokenPricingTimeout), the function reverts. However, this check only ensures that the data is not too old according to the specified threshold, without considering the possibility of the data being stale or delayed.
2. The code does not explicitly consider cases where the updatedAt timestamp indicates that the data is up to date, but in reality, the data might still be delayed or not reflecting the latest market conditions.

   Impact

   The lack of a comprehensive check for stale or delayed data can have significant implications. If the Chainlink Oracle experiences delays in updating its price feed, the function might continue to use outdated data, leading to incorrect pricing information being used in the smart contract logic. This could result in inaccurate decisions and actions based on the token prices obtained from the Chainlink Oracle.

   Code Snippet

   ( https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/oracles/providers/ChainlinkOracle.sol#L103-L124 )

   Tool used

Manual Review

Recommendation

To address this issue, it's essential to implement a thorough check for both data freshness and staleness in the getPriceInEth function. This can be achieved by comparing the updatedAt timestamp with both the current block's timestamp and a maximum allowable data age. Here's how to mitigate the vulnerability:

function getPriceInEth(address token) external returns (uint256) {
    ChainlinkInfo memory chainlinkOracle = _getChainlinkInfo(token);

    (uint80 roundId, int256 price, , uint256 updatedAt, ) = chainlinkOracle.oracle.latestRoundData();
    uint256 timestamp = block.timestamp;
    uint256 oracleStoredTimeout = uint256(chainlinkOracle.pricingTimeout);
    uint256 tokenPricingTimeout = oracleStoredTimeout == 0 ? DEFAULT_PRICING_TIMEOUT : oracleStoredTimeout;

    // Check if the data is stale (updated more than tokenPricingTimeout seconds ago)
    if (
        roundId == 0 ||
        price <= 0 ||
        updatedAt == 0 ||
        updatedAt < timestamp - tokenPricingTimeout
    ) {
        revert InvalidDataReturned();
    }

    // Check if the data is too outdated (updatedAt timestamp is too far in the past)
    uint256 maxDataAge = 1 hours; // Adjust as needed
    if (updatedAt < timestamp - maxDataAge) {
        revert StaleDataReturned();
    }

    // Rest of the function remains unchanged
    // ...
}

By introducing the maxDataAge check, the function will not only ensure that the data is not too old but will also flag data that, while technically within the freshness threshold, is too outdated to be considered reliable. This enhancement helps in guarding against the usage of potentially inaccurate or stale price data in the smart contract's decision-making process.

[sherlock-admin2]

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

Trumpero commented:

invalid/low, the code already validated the chainlink results, and issues related to chainlink round completeness is invalid

YakuzaKiawe commented:

invalid. See here