15,000 USDC
View results
Submission Details
Severity: medium
Valid

Assumption about oracle price that will have 8 decimals

Summary

In getTokenAmountFromUsd and getUsdValue there is the assumption that the returned price will have 8 decimals.

Vulnerability Details

In the current implementation, it assumes it in two spots:

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L346

AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L364

AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);

However, there are tokens with USD price feed's decimal is not equal to 8 like AMPL / USD feed decimals = 18).

Impact

When the price feed with decimals != 8 is set, the attacker can deposit a small amount of the asset and drain all the funds from the protocol.

Tool used

Manual Review

Recommendation

There are a couple of recommendations.

One recommendation can be in the corresponding functions of DSCEngine.sol to add a require that the decimals must be equal to 8.

AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
require(priceFeed.decimals == 8, "Not supported token");

An other recommendation can be to round up to 18 decimals all the priceFeed but you have to keep in mind in the future if there is going to be any priceFeed with more than 18 decimals.

Support

FAQs

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