sherlock-audit / 2023-05-ironbank-judging

2 stars 2 forks source link

Ignite - Insufficient Price Oracle Validation #333

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Ignite

medium

Insufficient Price Oracle Validation

Summary

The current implementation of PriceOracle.sol#getPriceFromChainlink() lacks a freshness check, which can result in the usage of stale prices.

Vulnerability Detail

Based on https://docs.chain.link/docs/historical-price-data, the following steps can be taken to avoid using a stale price returned by the Chainlink price feed.

  1. roundId and answeredInRound are also returned. "You can check answeredInRound against the current roundId. If answeredInRound is less than roundId, the answer is being carried over. If answeredInRound is equal to roundId, then the answer is fresh."

  2. 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.

function getPriceFromChainlink(address base, address quote) internal view returns (uint256) {
    (, int256 price,,,) = registry.latestRoundData(base, quote);
    require(price > 0, "invalid price");

    // Extend the decimals to 1e18.
    return uint256(price) * 10 ** (18 - uint256(registry.decimals(base, quote)));
}

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/oracle/PriceOracle.sol#L66-L72

The lack of necessary checks when using the latestRoundData() function implies that the returned price may be incorrect.

Impact

This could lead to incorrect or stale prices, which may result in invalid calculations.

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/oracle/PriceOracle.sol#L66-L72

Tool used

Manual Review

Recommendation

Add the following checks:

function getPriceFromChainlink(address base, address quote) internal view returns (uint256) {
-    (, int256 price,,,) = registry.latestRoundData(base, quote);
+    (uint80 roundId, int256 price, , uint256 updatedAt, uint80 answeredInRound) = registry.latestRoundData(base, quote);
     require(price > 0, "invalid price");
+    require(answeredInRound >= roundId, "price is stale");
+    require(updatedAt > 0, "round is incomplete");        

     // Extend the decimals to 1e18.
     return uint256(price) * 10 ** (18 - uint256(registry.decimals(base, quote)));
}

Duplicate of #9