Summary
The getUsdValue()
and getTokenAmountFromUsd()
can return incorrect values/amounts due to incorrect precision scaling.
Vulnerability Details
Below presents the getUsdValue()
and getTokenAmountFromUsd()
that can return incorrect values/amounts.
Two significant bugs:
Both functions receive the price
from Chainlink's price feed aggregators and presume that the received price
must always be represented using 8 decimals. Nevertheless, the Chainlink aggregators can actually represent the fed price data in different decimals for different collateral tokens.
Collateral tokens can have different decimals (e.g., WETH: 18, USDC: 6, GUSD (Gemini dollar): 2, and YAMv2: 24). Nonetheless, both functions treat all collateral tokens with 18 decimals.
Consequently, both functions can scale the received price
's offset precision to reflect its underlying token's decimals incorrectly.
function getUsdValue(address token, uint256 amount) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
@> return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
}
function getTokenAmountFromUsd(address token, uint256 usdAmountInWei) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
@> return (usdAmountInWei * PRECISION) / (uint256(price) * ADDITIONAL_FEED_PRECISION);
}
-
getUsdValue()
: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L366
-
getTokenAmountFromUsd()
: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L347
Impact
The getUsdValue()
and getTokenAmountFromUsd()
can scale the received price
's offset precision to reflect its underlying token's decimals incorrectly, resulting in incorrectly calculated value/amount returns.
To elaborate on the vulnerability. Assume that the XMock token is used as collateral and the price
received from Chainlink is represented in 8 decimals.
The XMock token has:
20 decimals
$100 in price
When a user deposits 5 XMock for minting DSC tokens, the vulnerable getUsdValue()
will return the following:
getUsdValue() returns:
((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
= (100 * 1e8) * 1e10 * (5 * 1e20) / 1e18
= 500 * 1e20
As you can see, the getUsdValue()
will return (500 * 1e20), not (500 * 1e18). Therefore, 50,000 DSC tokens can be minted instead of 500 tokens. This vulnerability will cause the DSC token depeg to $0 finally.
Tools Used
Manual Review
Recommendations
To fix the vulnerability, I recommend improving both getUsdValue()
and getTokenAmountFromUsd()
to scale the received price
's offset precision to reflect its underlying token's decimals, as shown below.
I introduce the _scalePrecision()
as a helper function for scaling the price
/amount
to an expected precision before working on it.
// getUsdValue()
function getUsdValue(address token, uint256 amount) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
- return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
+ uint8 chainlinkTokenDecimals = priceFeed.decimals();
+ uint8 tokenDecimals = token.decimals();
+ uint8 dscDecimals = i_dsc.decimals();
+ uint256 priceAfterAdjustPrecision = _scalePrecision(uint256(price), chainlinkTokenDecimals, dscDecimals);
+ uint256 amountAfterAdjustPrecision = _scalePrecision(amount, tokenDecimals, dscDecimals);
+ return (priceAfterAdjustPrecision * amountAfterAdjustPrecision) / (10 ** uint256(dscDecimals));
}
// getTokenAmountFromUsd()
function getTokenAmountFromUsd(address token, uint256 usdAmountInWei) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
- return (usdAmountInWei * PRECISION) / (uint256(price) * ADDITIONAL_FEED_PRECISION);
+ uint8 chainlinkTokenDecimals = priceFeed.decimals();
+ uint8 tokenDecimals = token.decimals();
+ uint8 dscDecimals = i_dsc.decimals();
+ uint256 priceAfterAdjustPrecision = _scalePrecision(uint256(price), chainlinkTokenDecimals, tokenDecimals);
+ uint256 usdAmountAfterAdjustPrecision = _scalePrecision(usdAmountInWei, dscDecimals, tokenDecimals);
+ return (usdAmountAfterAdjustPrecision * (10 ** uint256(tokenDecimals))) / priceAfterAdjustPrecision;
}
// _scalePrecision()
+ function _scalePrecision(
+ uint256 _value,
+ uint8 _valueDecimals,
+ uint8 _expectedDecimals
+ ) internal pure returns (uint256) {
+ if (_valueDecimals < _expectedDecimals) {
+ return _value * (10 ** uint256(_expectedDecimals - _valueDecimals));
+ } else if (_valueDecimals > _expectedDecimals) {
+ return _value / (10 ** uint256(_valueDecimals - _expectedDecimals));
+ }
+ return _value;
+ }