First Flight #21: KittyFi
First Flight #21
Beginner FriendlyDeFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

[H-2] Wrong calculation in `getUserVaultMeowllateralInEuros()`

nilay27

Description:

In getUserVaultMeowllateralInEuros() we want to return the value of user assets in EURO as KittyCoin is pegged to EURO.

However, the value of collaterals is fetched in USD as AggregatorV3Interface always returns value the price values in USD via latestRoundData().

The issue is that as we need to convert USD to EURO, we need to divide the collateral value with the euroPriceFeedAns as euroPriceFeedAns is also in terms of USD, however we are multiplying the value with EUROS instead of dividing.

Moreover, this function will always return the answer in token decimals. If the collateral has 8 decimals, it will return the answer in 8 decimals, and the same will be true for the token with 18 decimals.

This is highly dangerous as KittyPool uses this to mint and burn tokens, thus the value returned should be consistent in decimals.

Example with calculation shown in the code snippet below:

function getUserVaultMeowllateralInEuros(address _user) external view returns (uint256) {
(, int256 collateralToUsdPrice, , , ) = i_priceFeed.latestRoundData();
(, int256 euroPriceFeedAns, , ,) = i_euroPriceFeed.latestRoundData();
// @audit -> no checks for price staleness and price > 0;
uint256 collateralAns = getUserMeowllateral(_user).mulDiv(uint256(collateralToUsdPrice) * EXTRA_DECIMALS, PRECISION);
// @audit : potentially wrong calculation -> 1Euro = 1.09Dollar
/**
* if getUserMeowllaeral = 1e18, collateralToUsdPrice = 2e8
* collateralAns = 1e18 * (2e8 * 1e10)/1e18 = 2e18 in USD
* if euroPriceFeedAns is 1.09e8 (current value of euro to USD)
* in Euro it should be: 2e18/1.09e8 and not 2e18 * (1.09e8 * 1e10)/1e18 i.e 2.18e18
*/
return collateralAns.mulDiv(uint256(euroPriceFeedAns) * EXTRA_DECIMALS, PRECISION);
}

Impact:

The value of getUserVaultMeowllateralInEuros() will always be inflated by a factor of (EURO/USD)^2, thus always allocating the wrong amount of tokens or burning incorrect tokens

Proof of Concept:

Add the following test to KittyFiTest.t.sol:

function test_incorrectGetUserVaultMeowllateralInEuros() public userDepositsCollateral{
uint256 totalDepositedInVault = 5 ether;
uint256 userMeowllateralInEuros = wethVault.getUserVaultMeowllateralInEuros(user);
console.log("userMeowllateralInEuros", userMeowllateralInEuros);
​
// fetch ETH price from Chainlink Oracle:
(, int256 ethUSDPrice, , , ) = ethUsdPriceFeed.latestRoundData();
console.log("ethUSDPrice", uint256(ethUSDPrice));
​
// fetch EURO in terms of USD
AggregatorV3Interface euroPriceFeed = AggregatorV3Interface(config.euroPriceFeed);
(, int256 euroPrice, , , ) = euroPriceFeed.latestRoundData();
console.log("euroPrice", uint256(euroPrice));
​
// expected (collateralDeposited * usdPrice)/(EuroPrice)
uint256 expectedMeowllateralInEuros = totalDepositedInVault.mulDiv(uint256(ethUSDPrice), uint256(euroPrice));
console.log("expectedMeowllateralInEuros", expectedMeowllateralInEuros);
assertNotEq(expectedMeowllateralInEuros, userMeowllateralInEuros);
}

Console Log:

userMeowllateralInEuros 13267611847726776000000 // ~1.09^2 times expected value
ethUSDPrice 242854221843
euroPrice 109264000
expectedMeowllateralInEuros 11113185580017206033094

Recommended Mitigation:

Change the function to:

function getUserVaultMeowllateralInEuros(address _user) external view returns (uint256) {
(, int256 collateralToUsdPrice, , , ) = i_priceFeed.latestRoundData();
(, int256 euroPriceFeedAns, , ,) = i_euroPriceFeed.latestRoundData();
uint256 collateralAns = getUserMeowllateral(_user).mulDiv(uint256(collateralToUsdPrice) * EXTRA_DECIMALS, PRECISION);
- return collateralAns.mulDiv(uint256(euroPriceFeedAns) * EXTRA_DECIMALS, PRECISION);
+ return collateralAns.mulDiv(10**8, uint256(euroPriceFeedAns));
}
Updates

Lead Judging Commences

shikhar229169 Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`getUserVaultMeowllateralInEuros` performs incorrect conversion from usdc to euro

`getUserVaultMeowllateralInEuros` doesn't considers the collateral decimals, instead uses constant precision which works only for 18 decimals

Support

FAQs

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