Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Incorrect assumption about price feed decimals puts funds at risk

sobieski

Summary

DSCEngine contract arbitrarily assumes that the price feeds will have 8 decimals. If the token basket contains tokens for which the price feed has different number of decimals, the price calculation will be wrong, resulting in the system malfuction.

Vulnerability Details

The DSCEngine contract uses the following formula for calculating the USD value of a token:

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

The ADDITIONAL_FEED_PRECISION constant is used to adjust the price returned from the oracle to 18 decimals. This constant is currently hardcoded to 1e10, as the contract assumes the price feed will have 8 decimals.

uint256 private constant ADDITIONAL_FEED_PRECISION = 1e10;

While this holds true for the token basket used in the deploy script (WETH and WBTC), the documentation states:

The system is meant to be such that someone could fork this codebase, swap out WETH & WBTC for any basket of assets they like, and the code would work the same.

This assumption is not true for all of the price feeds. As per ChainLink documentation, an example of price feed that will break this assumption is AMPL/USD price feed on Ethereum Mainnet, which has 18 decimals.

Impact

Using incorrect decimals number in price calculation can lead to severe issues, including users taking debts unproportional to their collaterals. Please consider the following scenario:

  1. The Foundry DeFi Stablecoin codebase is forked and deployed with the WAMPL token included in the assets basket. The AMPL/USD price is at 11 USD at the moment.

  2. The attacker deposits the collateral of 1 AMPL. Because the system assumes that the oracle price feed for AMPL/USD has 8 decimals, it will multiply the price by ADDITIONAL_FEED_PRECISION = 1e10 in the USD value calculation. The actual number of decimals for AMPL/USD feed is 18. The collateral value in USD for the attacker should be 11 USD, but it will be 1e10 times bigger - 110 000 000 000 USD.

  3. The attacker is now able to mint up to 55 billion DSC, even though his real collateral value is only 11 USD.

Tools Used

Manual Review

Recommendations

Consider implementing the additional state variable, which will be a mapping holding the additional feed precisions for each of the token in the basket:

mapping(address token => uint256 additionalFeedPrecision) private s_additionalFeedPrecisions;

The mapping could be initialized in the constructor as such:

constructor(address[] memory tokenAddresses, address[] memory priceFeedAddresses, address dscAddress, uint256[] memory additionalPrecisions) {
// USD Price Feeds
if (tokenAddresses.length != priceFeedAddresses.length) {
revert DSCEngine__TokenAddressesAndPriceFeedAddressesMustBeSameLength();
}
if (tokenAddresses.length != additionalPrecisions.length) {
revert DSCEngine__TokenAddressesAndAdditionalPrecisionsMustBeSameLength();
}
// For example ETH / USD, BTC / USD, MKR / USD, etc
for (uint256 i = 0; i < tokenAddresses.length; i++) {
s_priceFeeds[tokenAddresses[i]] = priceFeedAddresses[i];
s_additionalFeedPrecisions[tokenAddresses[i]] = additionalPrecisions[i];
s_collateralTokens.push(tokenAddresses[i]);
}
i_dsc = DecentralizedStableCoin(dscAddress);
}

This mapping could be then utilized in the calculations instead of an arbitrary ADDITIONAL_FEED_PRECISION constant.

function getUsdValue(address token, uint256 amount) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
// 1 ETH = $1000
// The returned value from CL will be 1000 * 1e8
return ((uint256(price) * s_additionalFeedPrecisions[token]) * amount) / PRECISION;
}

Support

FAQs

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