SHOULD CHECK RETURN DATA FROM CHAINLINK AGGREGATORS

[code423n4]

Handle

defsec

Vulnerability details

Impact

The getPrice function in the contract ChainlinkAdapter.sol fetches the asset price from a Chainlink aggregator using the latestRoundData function. However, there are no checks on roundID nor timeStamp, resulting in stale prices. Stale prices could put funds at risk. According to Chainlink's documentation, This function does not error if no answer has been reached but returns 0, causing an incorrect price fed to the PriceOracle. The external Chainlink oracle, which provides index price information to the system, introduces risk inherent to any dependency on third-party data sources. For example, the oracle could fall behind or otherwise fail to be maintained, resulting in outdated data being fed to the index price calculations of the AMM. Oracle reliance has historically resulted in crippled on-chain systems, and complications that lead to these outcomes can arise from things as simple as network congestion.

Proof of Concept

1. Navigate to "https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-cssr/contracts/adapter/ChainlinkAdapter.sol#L49". 2. getPrice function has been used in the repository.
2. The following code does not check return value for the aggregator.
3. The sample can be seen below.

        (, int256 price, , , ) = feed[_asset].latestRoundData();
        uint256 decimalSum = feed[_asset].decimals() +
            IERC20Metadata(_asset).decimals();

Tools Used

Code Review

Recommended Mitigation Steps

Consider to add checks on the return data with proper revert messages if the price is stale or the round is incomplete, for example:

(uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = ETH_CHAINLINK.latestRoundData();
require(price > 0, "Chainlink price <= 0"); 
require(answeredInRound >= roundID, "...");
require(timeStamp != 0, "...");

[r2moon]

duplicated with #87