getTokenAmountFromUsd calculatations are wrong while using tokens with less than 18 decimals. It is impossible to liquidate users even if they don't have enough collateral.
Vulnerability Details
During liquidation, getTokenAmountFromUsd is called as shown below:
Let's see what happens if collateral is the address of WBTC (just one example) token which has 8 decimals:
function getTokenAmountFromUsd(address token, uint256 usdAmountInWei) public view returns (uint256) {
// price of ETH (token)
// $/ETH ETH ??
// $2000 / ETH. $1000 = 0.5 ETH
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
// ($10e18 * 1e18) / ($2000e8 * 1e10)
return (usdAmountInWei * PRECISION) / (uint256(price) * ADDITIONAL_FEED_PRECISION);
usdAmountInWei has 18 decimals, PRECISION has 18 decimals, uint256(price) has 8 decimals and ADDITIONAL_FEED_PRECISION has 10 decimals. Hence the return value of this function will be in 18 decimals. Which makes tokenAmountFromDebtCovered 18 decimal. Since WBTC has 8 decimal, further calculations will continue with 10^10 times more value than expected. Since this value is more than WBTC supply, liquidate function will always revert if WBTC is used as a collateral to liquidate.
Impact
There is no way to liquidate using WBTC or any other collateral that have less than 18 decimals as collateral. If we continue from WBTC example, when WBTC price is dropped users that hold WBTC as collateral won't be liquidateable and DSC will lose is peg and will be undercollateralized which lets to protocol insolvency. Since there is a known issue that "If the protocol ever becomes insolvent, there is almost no way to recover.", this problem will cause protocol to collapse. Holders of DSC will lose their funds because of depeg, hence this is a high level issue.
Tools Used
Manuel Review
Recommendations
Check token's decimal value in getAmountFromUsd and do further calculations accordingly.
There is no way to liquidate a user when they use protocol with tokens that have less than 18 decimals.
Severity
High Risk
Relevant GitHub Links
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol/#L229-L262
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol/#L340-L348
Summary
getTokenAmountFromUsdcalculatations are wrong while using tokens with less than 18 decimals. It is impossible to liquidate users even if they don't have enough collateral.Vulnerability Details
During liquidation,
getTokenAmountFromUsdis called as shown below:Let's see what happens if collateral is the address of WBTC (just one example) token which has 8 decimals:
usdAmountInWeihas 18 decimals,PRECISIONhas 18 decimals,uint256(price)has 8 decimals andADDITIONAL_FEED_PRECISIONhas 10 decimals. Hence the return value of this function will be in 18 decimals. Which makestokenAmountFromDebtCovered18 decimal. Since WBTC has 8 decimal, further calculations will continue with 10^10 times more value than expected. Since this value is more than WBTC supply,liquidatefunction will always revert if WBTC is used as a collateral to liquidate.Impact
There is no way to liquidate using WBTC or any other collateral that have less than 18 decimals as collateral. If we continue from WBTC example, when WBTC price is dropped users that hold WBTC as collateral won't be liquidateable and DSC will lose is peg and will be undercollateralized which lets to protocol insolvency. Since there is a known issue that "If the protocol ever becomes insolvent, there is almost no way to recover.", this problem will cause protocol to collapse. Holders of DSC will lose their funds because of depeg, hence this is a high level issue.
Tools Used
Manuel Review
Recommendations
Check token's decimal value in
getAmountFromUsdand do further calculations accordingly.