The Standard

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

Protocol would operate with massively flawed data when an accepted token gets removed

Proof of Concept

First, note that from the readMe of the contest we can see that protocol currently supports quite a number of tokens includong PAXG, native ETH, ARB, LINK... now from the discord chat, sponsor has relayed that the accepted tokens are going to be managed by them, coupling this with the OOS TokenManager we can see that there is where the removal/addition of tokens logic is implemented.

Now, take a look at SmartVaultV3.sol#L66-L73

function euroCollateral() private view returns (uint256 euros) {
//@audit
ITokenManager.Token[] memory acceptedTokens = getTokenManager().getAcceptedTokens();
for (uint256 i = 0; i < acceptedTokens.length; i++) {
ITokenManager.Token memory token = acceptedTokens[i];
euros += calculator.tokenToEurAvg(token, getAssetBalance(token.symbol, token.addr));
}
}

Note that this function is called for multiple other important logic, such as the maxMintable(), status() and even while calculating the calculateMinimumAmountOut().

Now case with this is that, this logic only includes accepted tokens, i.e when protocol decides to remove a token from it's list, the amount of the euroCollateral would be way off than it should really be, affecting all linked logic.

Impact

As stated in ## Proof of Concept once an asset(s) is delisted the value of euroCollateral would be way off (lesser than it really is) than it should be, using the simplest case of one asset to explain this, is the fact that even if protocol has previously been deposited with $10,000 worth of this assets's value, it's not going to be added to the euroCollateral() since getAssetBalamce() is not queried on this asset.

Other related impacts to be considerd, woild be fact that

  • maxMintable is now a lwer value than it should really be

  • The above leads to a scenario where a vault can now become under collaterized, since the check employed in undercollateralised() is minted > maxMintable();... essentially leading to a case where users are unfairly liquidated

  • Might lead to the inability to remove collateral since currentmintable would now be a smaller value.

  • This might even lead to calculateMinimumAmountOut being 0, due to this check: collateralValueMinusSwapValue >= requiredCollateralValue becoming true.

  • The same bug case form the LiquidationPool.sol would cause a user to unfairly lose out on their rewards when they try to claimRewards()

Recommended Mitigation Steps

The idea of euroCollateral ahould be rethought and not only assets that are presently accepted should be considered, but rather even previously accpeted assets.

Updates

Lead Judging Commences

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

remove-token

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

removetoken-low

Support

FAQs

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