Closed codehawks-bot closed 1 year ago
Invalid.
staleCheckLatestRoundData
returns in 8 decimals.
https://etherscan.io/address/0x5f4ec3df9cbd43714fe2740f5e3616155c5b8419#readContract
getUsdValue
is designed to return in 18 decimals and implemented correctly.
Due to wrong scaling, the returned-value (price) from the DSCEngine#
getUsdValue()
would be1e4
times larger than the actual valueSeverity
High Risk
Relevant GitHub Links
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L70-L71
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L356
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L363
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L366
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/libraries/OracleLib.sol#L26-L27
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/libraries/OracleLib.sol#L32
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L364-L365
Summary
Due to wrong scaling, the returned-value (price) from the DSCEngine#
getUsdValue()
would be1e4
times larger than the actual value.This lead to that the incremented-value of the
totalCollateralValueInUsd
would also be1e4
times larger than the actual value.Vulnerability Details
Within the DSCEngine, the
ADDITIONAL_FEED_PRECISION
and thePRECISION
would be defined. Then,1e10
and1e18
would be stored into there like this: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L70-L71Within the DSCEngine#
getAccountCollateralValue()
, the DSCEngine#getUsdValue()
would be called to increment thetotalCollateralValueInUsd
like this: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L356Within the DSCEngine#
getUsdValue()
, the collateral tokenprice
in USD would be returned via the OracleLib#staleCheckLatestRoundData()
. And then, it would be calculated and returned as the collateral token price in USD would be returned like this: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L363 https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L366The OracleLib#
staleCheckLatestRoundData()
, theanswer
would be retrieved via the the Chainlink's AggregatorV3Interface#latestRoundData()
and it would be returned as the price like this: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/libraries/OracleLib.sol#L26-L27 https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/libraries/OracleLib.sol#L32According to the NatSpec of the DSCEngine#
getUsdValue()
(at the line of the DSCEngine.sol#L364-L365),1000 * 1e8
would be expected as a returned-value when1 ETH = $1000
like this:According to the Chainlink Price Feed, the decimals precision of returned-value (
answer
) of both ETH/USD and BTC/USD from the Chainlink's AggregatorV3Interface#latestRoundData()
would be1e2
like this:However, within the DSCEngine#
getUsdValue()
above, the scaling of returned-value, which is retrieved as theanswer
from the Chainlink's AggregatorV3Interface#latestRoundData()
via the OracleLib#staleCheckLatestRoundData()
, would be wrong. Due to the wrong scaling, the returned-value (price) from the DSCEngine#getUsdValue()
would be1e4
times larger than the actual value.This lead to that the incremented-value of the
totalCollateralValueInUsd
would also be1e4
times larger than the actual value.Here is a scenario:
Scenario). At the line of the DSCEngine.sol#L366 in the DSCEngine#
getUsdValue()
. If we assume that $WETH would be used as a collateral token and theprice
of WETH is at $1000/WETH andamount
assigned is 1 WETH, the calculation process and result like this:As you can see the calculation process and result above, the returned-value is
1000 * 1e12
. This returned-value was supposed to be1000 * 1e8
according to the NatSpec of the DSCEngine#getUsdValue()
(at the line of the DSCEngine.sol#L365) above. Based on above, the returned-value from the DSCEngine#getUsdValue()
would be1e4
times larger than the actual value.Impact
This lead to that the incremented-value of the
totalCollateralValueInUsd
would also be1e4
times larger than the actual value.Tools Used
Recommendations
Within the DSCEngine, consider modifying the stored-value of the
ADDITIONAL_FEED_PRECISION
from1e10
and1e14
like this: