Summary
It is said that the protocol can be used for any ERC20 token whose price feed exists in Chainlink. But the current code base is not applying to a price feed whose decimals are not 8. A lot of tokens’ price feed decimals are not 8, for example, AMPL, BNB, DAI, etc.
Vulnerability Details
In the constructor function, you can set the price feed, but it doesn't consider the case that the price feed is not 8 decimals.
As a result of inputting a Chainlink price feed which is not 8 decimals. The protocol's calculation will fail to reflect the right price from the price feed.
Impact
The protocol will fail and DSC will de-peg because you can use much less collateral to mint much more token to dump the price in the second market. As a result, the protocol will fail.
Tools Used
Manual review
Recommendations
There are 2 ways to mitigate this vulnerability.
Let the protocol only accept the token whose price feed is 8 decimals.
...
error DSCEngine__PriceFeedMustBeEightDecimals();
...
constructor(address[] memory tokenAddresses, address[] memory priceFeedAddresses, address dscAddress) {
// USD Price Feeds
if (tokenAddresses.length != priceFeedAddresses.length) {
revert DSCEngine__TokenAddressesAndPriceFeedAddressesMustBeSameLength();
}
// For example ETH / USD, BTC / USD, MKR / USD, etc
for (uint256 i = 0; i < tokenAddresses.length; i++) {
+ if (AggregatorV3Interface(priceFeedAddresses[i]).decimals() != 8) {
+ revert DSCEngine__PriceFeedMustBeEightDecimals();
+ }
s_priceFeeds[tokenAddresses[i]] = priceFeedAddresses[i];
s_collateralTokens.push(tokenAddresses[i]);
}
i_dsc = DecentralizedStableCoin(dscAddress);
}
The optimal solution might be to change how the price is calculated within the protocol, enabling it to adapt to any price feed.
...
- uint256 private constant ADDITIONAL_FEED_PRECISION = 1e10;
...
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]);
+ uint256 decimals = priceFeed.decimals();
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
// ($10e18 * 1e18) / ($2000e8 * 1e10)
- return (usdAmountInWei * PRECISION) / (uint256(price) * ADDITIONAL_FEED_PRECISION); // @audit the problem here is we regard the default value of the decimals return by chainlink is 8. Sometimes it isn't. e.g., AMPL, BNB, DAI token.
+ return (usdAmountInWei * PRECISION) / uint256(price) * 10 ** (18 - decimals);
}
...
- function getAdditionalFeedPrecision() external pure returns (uint256) {
- return ADDITIONAL_FEED_PRECISION;
- }
function getUsdValue(address token, uint256 amount) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
+ uint256 decimals = priceFeed.decimals();
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
// 1 ETH = $1000
// The returned value from CL will be 1000 * 1e8
- return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
+ return ((uint256(price) * 10 ** (18 - decimals)) * amount) / PRECISION;
}
