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

Vault can become undercollateralized without warning if token removed from accepted list

t0x1c

Summary

To check if a vault is undercollateralized or not, the protocol makes use of the maxMintable() function which internally calls euroCollateral(). This particular function loops through the list of current getTokenManager().getAcceptedTokens() to find out the current collateral value in euro.

function euroCollateral() private view returns (uint256 euros) {
@---> 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));
}
}
​
function maxMintable() private view returns (uint256) {
@---> return euroCollateral() * ISmartVaultManagerV3(manager).HUNDRED_PC() / ISmartVaultManagerV3(manager).collateralRate();
}

The problem arises in the following scenario:

  • User deposits collateral using a token from the accepted list say, sUSD

  • User mints some amount of EUROs

  • TokenManager due to any reason, calls removeAcceptedToken() to remove sUSD from the list of accepted tokens

  • Immediately, the Vault becomes undercollateralized and can be liqudated causing the user to lose all of his collateral.

There is no warning system or grace period which can give the user a chance to deposit a new token or swap the current one for another acceptable one.

While removing a token may be okay for upcoming vaults to be created in future, for existing vaults it is certainly required to have a grace period of some sort.


This scenario also effects other functions like claimRewards(). All existing rewards associated with this token now vanish.

PoC

Apply the following patch to update test/smartVault.js and run via npx hardhat test --grep 'removing token causes undercollateralization' to see the test fail. The vault becomes undercollateralized as soon as the token is removed from accepted list.

diff --git a/test/smartVault.js b/test/smartVault_2.js
index 464b603..2e8ce3a 100644
--- a/test/smartVault.js
+++ b/test/smartVault_2.js
@@ -324,13 +324,25 @@ describe('SmartVault', async () => {
describe('swaps', async () => {
let Stablecoin;
+ let StandardTokenSymbol;
beforeEach(async () => {
Stablecoin = await (await ethers.getContractFactory('ERC20Mock')).deploy('sUSD', 'sUSD', 6);
const clUsdUsdPrice = 100000000;
const ClUsdUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('sUSD / USD');
await ClUsdUsd.setPrice(clUsdUsdPrice);
- await TokenManager.addAcceptedToken(Stablecoin.address, ClUsdUsd.address);
+ let tx = await TokenManager.addAcceptedToken(Stablecoin.address, ClUsdUsd.address);
+ let receipt = await tx.wait();
+ StandardTokenSymbol = receipt.events[0].args.symbol;
+ });
+
+ it('removing token causes undercollateralization', async () => {
+ await Stablecoin.mint(Vault.address, ethers.utils.parseEther('1'));
+ expect(await Vault.connect(user).mint(user.address, ethers.utils.parseEther('.8'))).not.to.be.reverted;
+ expect(await Vault.undercollateralised()).to.eq(false, "before token removal");
+
+ expect(await TokenManager.removeAcceptedToken(StandardTokenSymbol)).not.to.be.reverted;
+ expect(await Vault.undercollateralised()).to.eq(false, "after token removal"); // @audit : Vault becomes undercollateralized
});
it('only allows owner to perform swap', async () => {

Impact

  • Users lose funds with no warning system.

  • All existing rewards associated with the removed token vanish.

Tools Used

Hardhat

Recommendations

The protocol needs to think about having a two-step token removal system. They can initially mark a token as backlisted and give a grace period of X days to existing vault holders before they are affected.

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.