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

[L-01] Risk of incorrect calculation due to negative values in int256 conversion

maanvad3r

Summary

The smart contract DSCEngine.sol contains two functions, getTokenAmountFromUsd and getUsdValue, that fetch token prices from an external Oracle. The vulnerability lies in the incorrect calculation when converting a negative int256 value to uint256, which can lead to large positive numbers instead of the expected negative value.

Vulnerability Details

The functions getTokenAmountFromUsd and getUsdValue are designed to retrieve the price of a token in USD from an external Oracle, which returns the price as an int256 value. The vulnerability arises when these functions directly use uint256(price) in calculations without verifying whether the price is non-negative.

The issue occurs when the int256 price is negative. Converting a negative int256 to uint256 follows two's complement representation, resulting in a large positive value instead of the original negative value. As a consequence, the calculations produce unexpected and inaccurate results.

It's worth noting that the Chainlink Data Feeds use int instead of uint because some prices can be negative, for instance, when oil futures dropped below 0. However, in the provided code, the price obtained from the Chainlink Data Feed is converted from int256 to uint256 without checking if it's negative which can lead to incorrect calculations.

Reference: Stack Overflow link

Impact

The vulnerability can result in incorrect token amount and USD value calculations, leading to disruptions in the token's economic balance and potential financial losses for users. If the price is negative and mistakenly converted to a large positive value, users may receive excessive or insufficient token amounts, causing imbalances in the token's supply. Additionally, inaccurate USD value calculations can misrepresent token asset values, leading to incorrect financial decisions based on undervalued or overvalued assets. This can undermine user confidence in the contract's reliability and functionality.

POC

contract IncorrectCalculationPOC {
int256 negativePrice = -100; // Example negative price
​
function convertToInt() public view returns (uint256) {
uint256 convertedValue = uint256(negativePrice);
return convertedValue; // The result will be equal to 115792089237316195423570985008687907853269984665640564039457584007913129639836
}
}
​

Tools Used

  • VS Code

Recommendations

To address the issue make sure that the price variable is a non-negative value before converting it from a int256 into a uint256 value.

Here is an updated version for both of the functions:

  • https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L345-L347

function getTokenAmountFromUsd(address token, uint256 usdAmountInWei) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
​
// Ensure that the price is non-negative
require(price >= 0, "Price must be non-negative");
​
// Proceed with the calculation
return (usdAmountInWei * PRECISION) / (uint256(price) * ADDITIONAL_FEED_PRECISION);
}

  • https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L363-L366

function getUsdValue(address token, uint256 amount) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
​
// Ensure that the price is non-negative
require(price >= 0, "Price must be non-negative");
​
// Proceed with the calculation
return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
}

Support

FAQs

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