Cyfrin / 2023-07-foundry-defi-stablecoin

37 stars 32 forks source link

Decimal Assumption Vulnerability in `getUsdValue` Function #1113

Open codehawks-bot opened 1 year ago

codehawks-bot commented 1 year ago

Decimal Assumption Vulnerability in getUsdValue Function

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L361-L367

Summary

The function getUsdValue is assuming all collateral tokens have 8 decimals, which for these two [wbtc and weth] is fine but at the project description it says this protocol is supposed to be forked by others to add the collateral of their tastes, so when any other token with a different decimal than 8 be added it will deliver wrong values. Most devs who fork this repo will assume the math is correct and be exploited some time in the future just by adding a token with more or less decimals.

Vulnerability Details

366:        return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;

this line is assuming all tokens are 8 decimals since ADDITIONAL_FEED_PRECISION = 1e10, so when the project be forked as it aims to be or any other token be added with different decimal the numbers will break.

Impact

Incorrect asset valuation, leading to potential financial inaccuracies or losses.

Tools Used

Manual review

Recommendations

Implement dynamic decimal handling based on each token's specific decimal count.