sherlock-audit / 2022-09-knox-judging

0 stars 0 forks source link

berndartmueller - Chainlink's `latestRoundData` might return stale or incorrect results #137

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

berndartmueller

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:

Vulnerability Detail

The PricerInternal._latestAnswer64x64 function uses Chainlink's latestRoundData() to get the latest price. However, there is no check if the return value indicates stale data.

Impact

The PricerInternal could return stale price data for the underlying asset.

Code Snippet

PricerInternal._latestAnswer64x64

/**
  * @notice gets the latest price of the underlying denominated in the base
  * @return price of underlying asset as 64x64 fixed point number
  */
function _latestAnswer64x64() internal view returns (int128) {
    (, int256 basePrice, , , ) = BaseSpotOracle.latestRoundData();
    (, int256 underlyingPrice, , , ) =
        UnderlyingSpotOracle.latestRoundData();

    return ABDKMath64x64.divi(underlyingPrice, basePrice);
}

Tool Used

Manual review

Recommendation

Consider adding checks for stale data. e.g

(uint80 roundId, int256 basePrice, , uint256 updatedAt, uint80 answeredInRound) = BaseSpotOracle.latestRoundData();

require(answeredInRound >= roundId, "Price stale");
require(block.timestamp - updatedAt < PRICE_ORACLE_STALE_THRESHOLD, "Price round incomplete");
0xCourtney commented 1 year ago

https://github.com/KnoxFinance/knox-contracts/pull/85

rcstanciu commented 1 year ago

Reply from @arbitrary-CodeBeholder


looks good to me