Part 2

Summary

The Collateral.getPrice() function makes an unchecked external call to an IPriceAdapter contract to fetch the price of a collateral asset. If the priceAdapter address is invalid or the external call fails, the transaction will revert, potentially disrupting the system. This unchecked external call introduces risks such as denial of service and reliance on an untrusted oracle.

Vulnerability Details

Code Analysis

The getPrice() function is defined as follows:

Key Observations:

1. Unchecked External Call : The function directly calls IPriceAdapter(priceAdapter).getPrice() without any safeguards. If the priceAdapter contract reverts or behaves maliciously, the entire transaction will fail.

2. No Fallback Mechanism : There is no backup mechanism (e.g., caching the last valid price or using a secondary oracle) in case the external call fails.

3. Invalid Price Adapter Risk : While the function checks that priceAdapter is not zero, it does not validate whether the address points to a legitimate IPriceAdapter implementation. A malicious or misconfigured address could cause issues.

Attack Scenario

1. An attacker exploits a governance flaw or gains control over the priceAdapter address.

2. The attacker sets the priceAdapter to a malicious contract that always reverts when getPrice() is called.

3. Any function relying on getPrice() (e.g., getAdjustedPrice()) will fail, causing disruptions in collateral valuation and pricing.

4. Alternatively, the attacker sets the priceAdapter to an invalid address (e.g., a non-contract address), causing the external call to revert.

Impact

• Denial of Service : Functions relying on getPrice() will fail, rendering critical operations unusable.

• Financial Losses : Incorrect or unavailable pricing data could lead to improper valuations, affecting debt calculations, liquidations, and rewards.

• Oracle Dependency : The system becomes overly reliant on a single external oracle, introducing centralization risks.

Tools Used

1. Manual Code Review : Analyzed the getPrice() function and its interactions with the priceAdapter.

2. Slither : Static analysis tool used to identify unchecked external calls and potential vulnerabilities.

3. MythX : Security analysis platform used to verify risks associated with external calls and oracle dependencies.

Recommendations

Short-Term Fix

Add a fallback mechanism to handle external call failures gracefully. For example:

Introduce a cachedPrice field in the Collateral.Data struct to store the last valid price.

Long-Term Fix

1. Multiple Oracles : Implement a decentralized oracle system with multiple price feeds. Use a median or weighted average of prices to reduce reliance on a single source.

   function getPrice(Data storage self) internal view returns (UD60x18 priceX18) {

   UD60x18[] memory prices = new UD60x18[](self.priceAdapters.length);

   uint256 validPrices = 0;

   ​

   for (uint256 i = 0; i < self.priceAdapters.length; i++) {

   try IPriceAdapter(self.priceAdapters[i]).getPrice() returns (UD60x18 fetchedPrice) {

   prices[validPrices] = fetchedPrice;

   validPrices++;

   } catch {}

   }

   ​

   require(validPrices > 0, "No valid price feeds");

   priceX18 = Math.median(prices, validPrices);

   }

2. Price Validation : Add validation to ensure the fetched price is within an acceptable range compared to the last known price.

   require(

   fetchedPrice >= self.lastPrice.mul(ud60x18(0.9)) && fetchedPrice <= self.lastPrice.mul(ud60x18(1.1)),

   "Price out of bounds"

   );

3. Governance Controls : Restrict updates to the priceAdapter address to trusted entities using role-based access control (RBAC).