The Standard

The Standard
DeFiHardhat
20,000 USDC
View results
Submission Details
Severity: low
Invalid

Unhandled chainlink revert would lock all price oracle access

Summary

Call to latestRoundData could potentially revert and make it impossible to query any prices. Feeds cannot be changed after they are configured. so this would result in a permanent denial of service.

Vulnerability Details

Chainlink’s multisigs can immediately block access to price feeds at will. Therefore, to prevent denial of service scenarios, it is recommended to query Chainlink price feeds using a defensive approach with Solidity’s try/catch structure. In this way, if the call to the price feed fails, the caller contract is still in control and can handle any errors safely and explicitly.

// PriceCalculator::tokenToEurAvg method
(, int256 eurUsdPrice,,,) = clEurUsd.latestRoundData();
return collateralUsd / uint256(eurUsdPrice);

Impact

Unhandled chainlink revert would lock all price oracle access thereby preventing all forms of borrowing, debt position repayment and querrying of SmartVault status.

Tools Used

Manual review

Recommendations

Surround the call to latestRoundData() with try/catch instead of calling it directly, similar to how it's done in PriceCaclulator::avgPrice(). In a scenario where the call reverts, the catch block can be used to call a fallback oracle or handle the error in any other suitable way.

// PriceCalculator::eurToToken method
- (, int256 eurUsdPrice,,,) = clEurUsd.latestRoundData();
- return _eurValue * uint256(eurUsdPrice) / uint256(tokenUsdPrice) / 10 ** getTokenScaleDiff(_token.symbol, _token.addr);
+ try clEurUsd.latestRoundData() return (, int256 eurUsdPrice,,,) {
+ return _eurValue * uint256(eurUsdPrice) / uint256(tokenUsdPrice) / 10 ** getTokenScaleDiff(_token.symbol, _token.addr);
+ } catch {
+ // fallback strategy here
+ }

something similar should be done for all occurences of the price feed query as enumerated in the relevent links sections.

Had the calculator variable in contracts/SmartVaultV3::calculator address been modifiable after initial setup and deployment of the vault, then the impact of this issue could have been reduced by updating that to a new calculator with a different oracle and or price feed. Hence, providing a functionality to replace or updateing oracle feeds after they are configured is a nice to have functionality.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

chainlink-revert

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

chainlink-revert

Support

FAQs

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