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

Due to wrong scaling, the returned-value (price) from the DSCEngine#`getUsdValue()` would be `1e4` times larger than the actual value

0xmuxyz

Summary

Due to wrong scaling, the returned-value (price) from the DSCEngine#getUsdValue() would be 1e4 times larger than the actual value.

This lead to that the incremented-value of the totalCollateralValueInUsd would also be 1e4 times larger than the actual value.

Vulnerability Details

Within the DSCEngine, the ADDITIONAL_FEED_PRECISION and the PRECISION would be defined. Then, 1e10 and 1e18 would be stored into there like this:
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L70-L71

uint256 private constant ADDITIONAL_FEED_PRECISION = 1e10; /// @audit
uint256 private constant PRECISION = 1e18; /// @audit

Within the DSCEngine#getAccountCollateralValue(), the DSCEngine#getUsdValue() would be called to increment the totalCollateralValueInUsd like this:
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L356

function getAccountCollateralValue(address user) public view returns (uint256 totalCollateralValueInUsd) {
// loop through each collateral token, get the amount they have deposited, and map it to
// the price, to get the USD value
for (uint256 i = 0; i < s_collateralTokens.length; i++) {
address token = s_collateralTokens[i];
uint256 amount = s_collateralDeposited[user][token];
totalCollateralValueInUsd += getUsdValue(token, amount); /// @audit
}
return totalCollateralValueInUsd;
}

Within the DSCEngine#getUsdValue(), the collateral token price in USD would be returned via the OracleLib#staleCheckLatestRoundData(). And then, it would be calculated and returned as the collateral token price in USD would be returned like this:
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L363
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L366

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

The OracleLib#staleCheckLatestRoundData(), the answer would be retrieved via the the Chainlink's AggregatorV3Interface#latestRoundData() and it would be returned as the price like this:
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/libraries/OracleLib.sol#L26-L27
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/libraries/OracleLib.sol#L32

function staleCheckLatestRoundData(AggregatorV3Interface priceFeed)
public
view
returns (uint80, int256, uint256, uint256, uint80)
{
(uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) =
priceFeed.latestRoundData(); /// @audit
​
uint256 secondsSince = block.timestamp - updatedAt;
if (secondsSince > TIMEOUT) revert OracleLib__StalePrice();
​
return (roundId, answer, startedAt, updatedAt, answeredInRound); /// @audit
}

According to the NatSpec of the DSCEngine#getUsdValue() (at the line of the DSCEngine.sol#L364-L365), 1000 * 1e8 would be expected as a returned-value when 1 ETH = $1000 like this:

// 1 ETH = $1000
// The returned value from CL will be 1000 * 1e8

According to the Chainlink Price Feed, the decimals precision of returned-value (answer) of both ETH/USD and BTC/USD from the Chainlink's AggregatorV3Interface#latestRoundData() would be 1e2 like this:

  • ETH/USD：https://data.chain.link/ethereum/mainnet/crypto-usd/eth-usd

  • BTC/USD：https://data.chain.link/ethereum/mainnet/crypto-usd/btc-usd

However, within the DSCEngine#getUsdValue() above, the scaling of returned-value, which is retrieved as the answer from the Chainlink's AggregatorV3Interface#latestRoundData() via the OracleLib#staleCheckLatestRoundData(), would be wrong.
Due to the wrong scaling, the returned-value (price) from the DSCEngine#getUsdValue() would be 1e4 times larger than the actual value.

This lead to that the incremented-value of the totalCollateralValueInUsd would also be 1e4 times larger than the actual value.

Here is a scenario:

Scenario).
At the line of the DSCEngine.sol#L366 in the DSCEngine#getUsdValue().
If we assume that $WETH would be used as a collateral token and the price of WETH is at $1000/WETH and amount assigned is 1 WETH, the calculation process and result like this:

return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
return ((uint256(1000 * 1e2) * 1e10) * (1 * 1e18)) / 1e18;
return ((uint256(1000 * 1e12) * (1 * 1e18)) / 1e18;
return uint256(1000 * 1e30) / 1e18;
return uint256(1000 * 1e12);

As you can see the calculation process and result above, the returned-value is 1000 * 1e12. This returned-value was supposed to be 1000 * 1e8 according to the NatSpec of the DSCEngine#getUsdValue() (at the line of the DSCEngine.sol#L365) above.
Based on above, the returned-value from the DSCEngine#getUsdValue() would be 1e4 times larger than the actual value.

Impact

This lead to that the incremented-value of the totalCollateralValueInUsd would also be 1e4 times larger than the actual value.

Tools Used

  • Chainlink Data Feed：https://data.chain.link/

Recommendations

Within the DSCEngine, consider modifying the stored-value of the ADDITIONAL_FEED_PRECISION from 1e10 and 1e14 like this:

+ uint256 private constant ADDITIONAL_FEED_PRECISION = 1e14;
- uint256 private constant ADDITIONAL_FEED_PRECISION = 1e10;

Support

FAQs

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