The Standard

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

Portion of collateral remains locked due to incorrect calculation in `SmartVaultV3::canRemoveCollateral`

Description

SmartVaultV3::canRemoveCollateral is intended to validate whether _amount of _token can be safely withdrawn from the vault while maintaining its collateralization status. If the vault has no outstanding debt, then collateral can be removed freely; otherwise, the Euro value of the amount of collateral to remove cannot exceed the current mintable allowance, and the amount already minted cannot exceed the difference between these values while remaining fully collateralized. In other words, it should not be possible to withdraw more collateral than was deposited, and any outstanding debt must be covered by the corresponding required collateral amount.

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, this logic is not correct and will result in a portion of the collateral being locked so long as the minted amount is non-zero, even if it is not required to back the outstanding debt.

Impact

This issue has a medium severity since the protocol does not function as expected.

Proof of Concept

Considering a collateral rate of :

  1. Alice deposits 1000 EUR in LINK to her vault.

  2. maxMintable is 833.

  3. Alice mints 100 EURO.

  4. Alice attempts to remove 800 EUR in LINK.

  5. maxMintable exceeds the amount to remove (833 > 800) but minted exceeds the difference (100 > (833 - 800)), so removal is not allowed; however, the removal of 800 EUR in LINK (if succeeded) would leave 200 EUR in LINK. 200 EUR in LINK has a maxMintable of 166 EURO, which exceeds the minted value of 100 EURO, meaning that the removal of 800 EUR in LINK should succeed and leave a maxMintable of 166 EURO (of which 100 EURO has already been minted).

Recommended Mitigation

SmartVaultV3::canRemoveCollateral should potentially consider the value returned by SmartVaultV3::euroCollateral instead of SmartVaultV3::maxMintable, in conjunction with and depending on changes that rectify the conflation of LTV with the liquidation point/health factor.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

canRemoveCollateral

Support

FAQs

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