sherlock-audit / 2022-11-float-capital-judging

2 stars 1 forks source link

neila - `getRoundData()` Oracle data feed is insufficiently validated #39

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

neila

medium

getRoundData() Oracle data feed is insufficiently validated

Summary

Oracle data feed is insufficiently validated Found by: @Tomosuke0930

Vulnerability Detail

Important A read can revert if the caller is requesting the details of a round that was invalid or has not yet been answered. If you are deriving a round ID without having observed it before, the round might not be complete. To check the round, validate that the timestamp on that round is not 0.

Ref: https://docs.chain.link/docs/data-feeds/price-feeds/historical-data/#:~:text=getRoundData

If you call getRoundData(), you must check the timestamp is not 0

Impact

The round may not complete

Code Snippet

https://github.com/unchain-dev/2022-11-float-capital-UNCHAIN/blob/e167ac24613e02748aad72c1d216283225733355/contracts/keepers/OracleManagerUtils.sol#L60

  function getMissedEpochPriceUpdates(
    IOracleManager oracleManager,
    uint32 _latestExecutedEpochIndex,
    uint80 _previousOracleUpdateIndex,
    uint256 _numberOfUpdatesToTryFetch
  ) public view returns (uint80[] memory _missedEpochOracleRoundIds) {
    /* ~~~ */
    (, , uint256 _previousOracleUpdateTimestamp, , ) = chainlinkOracle.getRoundData(_previousOracleUpdateIndex);

    /* ~~~ */
      (, , uint256 _currentOracleUpdateTimestamp, , ) = chainlinkOracle.getRoundData(_missedEpochExecution._currentOracleRoundId);

    /* ~~~ */

    (, , _currentOracleUpdateTimestamp, , ) = chainlinkOracle.getRoundData(_missedEpochExecution._currentOracleRoundId);
  }
}

Tool used

Manual Review

Recommendation

Add checking like this

(, previousPrice, ,timestamp , ) = chainlinkOracle.getRoundData(latestExecutedOracleRoundId);
require(timestamp != 0,"Invalid Round");
JasoonS commented 1 year ago

Will ask the chainlink team about these to tripple check - but I don't think this is necessary.

Also we are checking those timestamps very clearly in this code: image