15,000 USDC
View results
Submission Details
Severity: medium
Valid

Chainlink price is used without checking validity

Vulnerability Details

The Cyfrin protocol relies on a Chainlink price oracle to determine the token amount and the USD value. However, the current implementation does not include any checks to verify the freshness of the price fetched from the Chainlink oracle.

File: src/DSCEngine.sol
340 function getTokenAmountFromUsd(address token, uint256 usdAmountInWei) public view returns (uint256) {
341 // price of ETH (token)
342 // $/ETH ETH ??
343 // $2000 / ETH. $1000 = 0.5 ETH
344 AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
345 (, int256 price,,,) = priceFeed.staleCheckLatestRoundData();//@audit chainlink price feed - stale price check is missing
346 // ($10e18 * 1e18) / ($2000e8 * 1e10)
347 return (usdAmountInWei * PRECISION) / (uint256(price) * ADDITIONAL_FEED_PRECISION);
348: }
361 function getUsdValue(address token, uint256 amount) public view returns (uint256) {
362 AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
363 (, int256 price,,,) = priceFeed.staleCheckLatestRoundData();//@audit //@audit chainlink price feed - stale price check is missing
364 // 1 ETH = $1000
365 // The returned value from CL will be 1000 * 1e8
366 return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
367: }

As we can see from the lines of code above, the staleCheckLatestRoundData() function is called in both cases to check the Chainlink Oracle for stale data.

File: src/libraries/OracleLib.sol
21 function staleCheckLatestRoundData(AggregatorV3Interface priceFeed)
22 public
23 view
24 returns (uint80, int256, uint256, uint256, uint80)
25 {
26 (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) =//@audit
27 priceFeed.latestRoundData();
28
29 uint256 secondsSince = block.timestamp - updatedAt;
30 if (secondsSince > TIMEOUT) revert OracleLib__StalePrice();
31
32 return (roundId, answer, startedAt, updatedAt, answeredInRound);
33: }

Although, the priceFeed is returning the following variables:

  • roundId

  • answer

  • startedAt

  • updatedAt

  • answeredInRound

, these return values are also meant to be used to do some extra checks before updating the price.
The absence of proper checks in the current implementation can result in potential issues when Chainlink initiates a new round and faces challenges in reaching a consensus on the oracle's new value. If these checks are not in place, consumers of this contract may continue to utilize outdated, stale, or inaccurate data in situations where oracles fail to submit and commence a new round. Some potential reasons for this could include scenarios where Chainlink nodes abandon the oracle, chain congestion occurs, or vulnerabilities/attacks target the Chainlink system.

Impact

This vulnerability is categorized as MEDIUM since it impacts user assets solely when the Chainlink oracle is in a compromised or unreliable state.

Tools Used

VSCode

Recommendations

To rectify this problem, it is advisable to incorporate checks that guarantee the freshness of the price received from Chainlink. You can utilize the following code snippet as a reference for validating the Chainlink price:

21 function staleCheckLatestRoundData(AggregatorV3Interface priceFeed)
22 public
23 view
24 returns (uint80, int256, uint256, uint256, uint80)
25 {
26 (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) =
27 priceFeed.latestRoundData();
28
+ require(price > 0, "Invalid chainlink price");
+ require(answeredInRound >= roundId, "Stale price");
29 uint256 secondsSince = block.timestamp - updatedAt;
30 if (secondsSince > TIMEOUT) revert OracleLib__StalePrice();
31
32 return (roundId, answer, startedAt, updatedAt, answeredInRound);
33: }

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.