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

Price feed is assumed to always return value with 8 decimals

Summary

I mark it as 'medium' , because the chance is very little, but if it occurs it will break the whole project logic.
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. The following sentence from the project documentation give us reason to be careful no to hardcode WETH & WBTC specific things in the engine. getUsdValue is using ChainLink datafeed, which in the current implementation is assuming that the return value will always be with 8 decimal places value. But in https://docs.chain.link/data-feeds/price-feeds/addresses we can see that AMPL/USD returns 18 decimal places result, which could result in larger confusion in the price calculations.

Vulnerability Details

PoC
We assume that:
1 AMPL = $100
Chainlink datafeed for AMPL/USD will then return
10 000 000 000 0000 000 000 = 100 * 18 decimal places
The current code formula

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

will return a value of 1 AMPL = 10 000 000 000 USD, which is faaar away from the truth -> 1 AMPL = 100 USD

Impact

Low likelihood with high bad impact on price conversions if it occurs.

Tools Used

Manual Review

Recommendations

Calculate ADDITIONAL_FEED_PRECISION the following way:

function getUsdValue(
address token,
uint256 amount
) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(
s_priceFeeds[token]
);
(, int256 price, , , ) = priceFeed.staleCheckLatestRoundData();
// If price feed of the token returns 18 decimals (as it is with AMPL/USD),
// then the additionFeedPRecision would be 0
uint256 additionalFeedPrecsision = PRECISION - priceFeed.decimals();
return
((uint256(price) * additionalFeedPrecsision) * amount) / PRECISION;
}
  • The same stands for getTokenAmountFromUsd()
    The additionalPrecision should be calculated from dataFeed.decimals()

Support

FAQs

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