Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Price access could get DOS'd since queries to chainlink are not done in a try/catch incase of reverts

bauchibred

Summary

Chainlink is heavily relied upon within DSC, infact it's the only oracle body being used, which exarcebates this issue.
Now it'd be key note that asides this blog from openzeppelin mentioning that it is possible that Chainlink’s "multisigs immediately block access to price feeds at will". Oracles can also be taken down for maintenance/safety reasons, which is why it's a pretty popular practise to wrap chainlink queries in a try/catch, and if the call fails for whatever reason the fallback mechanism is there to sort things out and prevent a denial of service from occurring when trying to access the price feed.

Vulnerability Details

See Summary

Additionally note that the query to chainlink reverting means that all instance of getting price is inacessible and all actions attached to them, for example the call to get the health factor would no longer be accessible.
See the getHealthFactor() function of
DSCEngine.sol:

function getHealthFactor(address user) external view returns (uint256) {
return _healthFactor(user);
}
​
​
/*
* Returns how close to liquidation a user is
* If a user goes below 1, then they can get liquidated
*/
function _healthFactor(address user) private view returns (uint256) {
(uint256 totalDscMinted, uint256 collateralValueInUsd) = _getAccountInformation(user);
return _calculateHealthFactor(totalDscMinted, collateralValueInUsd);
}
​
​
function _getAccountInformation(address user)
private
view
returns (uint256 totalDscMinted, uint256 collateralValueInUsd)
{
totalDscMinted = s_DSCMinted[user];
collateralValueInUsd = getAccountCollateralValue(user);
}
function getAccountCollateralValue(address user) public view returns (uint256 totalCollateralValueInUsd) {
// loop through each collateral token, get the amount they have deposited, and map it to
// the price, to get the USD value
for (uint256 i = 0; i < s_collateralTokens.length; i++) {
address token = s_collateralTokens[i];
uint256 amount = s_collateralDeposited[user][token];
totalCollateralValueInUsd += getUsdValue(token, amount);
}
return totalCollateralValueInUsd;
}
​

This would be iacessible since the getUsdValue() function would revert the call.

Tool used

Manual Audit

Recommend Mitigation

Use a try/catch block around the latestRoundData() calls. If these calls revert, the catch block should handle the failure accordingly. This can include a fallback mechanism, an alternative oracle call, or a contingency procedure to pause operations or any reasonable mechanism.

Support

FAQs

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