PriceOracle has invalid checks on price staleness.

mediumCode4rena

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/PriceOracle.sol#L124-L125

Vulnerability details

Impact

PriceOracle has invalid checks on price staleness.

Proof of Concept

There are two checks on price staleness in PriceOracle::getAssetPriceFromChainlink. But both checks are invalid. (1) updatedAt != 0 In chainlink aggregator, the price is updated at a set heartbeat and a threshold of deviation. updatedAt should be used to check if the answer is within the hearbeat or acceptable time limits. See doc.

(2) answeredInRound >= roundId answeredInRound is deprecated and shouldn't be used. see doc.

solidity📋
//src/PriceOracle.sol
  /// @notice Query the price of asset from chainlink oracle
  function getAssetPriceFromChainlink(address asset) public view returns (uint256) {
    AggregatorV2V3Interface sourceAgg = assetChainlinkAggregators[asset];
    require(address(sourceAgg) != address(0), Errors.ASSET_AGGREGATOR_NOT_EXIST);

    (uint80 roundId, int256 answer, , uint256 updatedAt, uint80 answeredInRound) = sourceAgg.latestRoundData();
    require(answer > 0, Errors.ASSET_PRICE_IS_ZERO);
|>  require(updatedAt != 0, Errors.ORACLE_PRICE_IS_STALE);
|>  require(answeredInRound >= roundId, Errors.ORACLE_PRICE_IS_STALE);

    return uint256(answer);
  }

(https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/PriceOracle.sol#L124-L125)

Tools Used

Manual

Recommended Mitigation Steps

Consider using asset-specific hearbeat(e.g. ETH/USD has 1 hour hearbeat) and check against (block.timestamp - updatedAt).

Assessed type

Oracle