The Standard

The Standard
DeFiHardhat
20,000 USDC
View results
Submission Details
Severity: medium
Valid

`currentMintable` and `eurValueToRemove` are not in the same unit to compare could prevent users from withdrawing collateral

Summary

The function canRemoveCollateral() compares two variables in different units, which prevents users from withdrawing collateral.

Vulnerability Details

The canRemoveCollateral() function checks if the vault still has enough collateral if it removes _amount of _token. It compares the current maximum mintable with the value of the removed token.

function maxMintable() private view returns (uint256) {
return euroCollateral() * ISmartVaultManagerV3(manager).HUNDRED_PC() / ISmartVaultManagerV3(manager).collateralRate();
}
function canRemoveCollateral(ITokenManager.Token memory _token, uint256 _amount) private view returns (bool) {
if (minted == 0) return true;
uint256 currentMintable = maxMintable();
uint256 eurValueToRemove = calculator.tokenToEurAvg(_token, _amount);
return currentMintable >= eurValueToRemove &&
minted <= currentMintable - eurValueToRemove;
}

However, these two variables are in different units. currentMintable is the value of the collateral token scaled by collateralRate, while eurValueToRemove is just the value of the removed token.

Impact

This issue may prevent users from withdrawing their collateral when they should be allowed to.

Consider the following scenario:

  1. The vault has minted only 1 EURO, and the collateral value in the vault is 1,000,000 EUROs. Assume the collateralRate is 120%.

  2. Since the vault only minted 1 EURO, it should only need a collateral value of 1.2 EUROs to cover it, so the user should be allowed to withdraw 999998.8 EUROs worth of collateral.

  3. However, in function canRemoveCollateral(), we will have currentMintable = 1e6 / 120% = 833333.333.

  4. As eurValueToRemove isn't scaled to collateralRate, any attempt to withdraw more than 833334 EUROs worth of collateral would fail.

Tools Used

Manual Review

Recommendations

Correct the implementation by scaling eurValueToRemove with collateralRate before performing the check.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

canRemoveCollateral

Support

FAQs

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

Give us feedback!