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

M-1 Owner of the vault can frontrun the liquidate tx, mint new EUROs and make undercollateralised() revert

agadzhalov

Summary

The owner of the vault can frontrun liquidate() by providing more amount of collateral to mint enough amount of EUROs and blocking the vaultManger of liquidating the smartVault

Vulnerability Details

The vaultManager always liquidates 100% of the smartVault. This way when the owner of the smartVault tracks the liqiudate() transaction in the mempool he can provide more amount of collateral and cause undercollateralised() to always revert.

Example:

There are 100 EUROs minted for the smartVault. The liquidator notices that is undercollateralized and decides to liquidate it. The owner realises that the vault will be liquidated and frontruns the liquidation transaction by providing enough amount of collateral and minting more EUROs. This way minted > maxMintable(); will always be true and undercollateralised() will revert.

PoC

in smartVaultManager.js you can execute this test:

it.only('owner frontruns manager during liquidation', async () => {
const protocolETHBalance = await protocol.getBalance();
const protocolUSDTBalance = await Tether.balanceOf(protocol.address);
await TokenManager.addAcceptedToken(Tether.address, ClUsdUsd.address);
const tetherValue = 1000000000;
const ethValue = ethers.utils.parseEther('1');
await Tether.mint(vaultAddress, tetherValue);
await user.sendTransaction({to: vaultAddress, value: ethValue});
const { maxMintable } = (await VaultManager.connect(user).vaults())[0].status;
const mintValue = maxMintable.mul(99).div(100);
const vault = await ethers.getContractAt('SmartVaultV3', vaultAddress);
await vault.connect(user).mint(user.address, mintValue)
​
let liquidate = VaultManager.connect(admin).liquidateVault(1);
await expect(liquidate).to.be.revertedWith('err-invalid-liquidator');
​
// shouldn't liquidate any vaults, as both are sufficiently collateralised, should revert so no gas fees paid
liquidate = VaultManager.connect(liquidator).liquidateVault(1);
await expect(liquidate).to.be.revertedWith('vault-not-undercollateralised');
liquidate = VaultManager.connect(liquidator).liquidateVault(2);
await expect(liquidate).to.be.revertedWith('vault-not-undercollateralised');
​
// drop price of eth to $1000, first vault becomes undercollateralised
await ClEthUsd.setPrice(100000000000);
​
liquidate = VaultManager.connect(liquidator).liquidateVault(2);
await expect(liquidate).to.be.revertedWith('vault-not-undercollateralised');
// first user's vault should be liquidated
​
// frontrun happens here
const moreCollateralAmount = ethers.utils.parseEther('0.6');
await user.sendTransaction({to: vaultAddress, value: moreCollateralAmount});
liquidate = VaultManager.connect(liquidator).liquidateVault(1);
// it is expected not to revert here but it will actually revert with reason 'vault-not-undercollateralised'
// because the owner of the vault provided more collateral in a frontrunning tx
await expect(liquidate).not.to.be.reverted;
​
await expect(liquidate).to.emit(VaultManager, 'VaultLiquidated').withArgs(vaultAddress);
const userVaults = await VaultManager.connect(user).vaults();
const otherUserVaults = await VaultManager.connect(otherUser).vaults();
expect(userVaults[0].status.liquidated).to.equal(true);
expect(otherUserVaults[0].status.liquidated).to.equal(false);
expect(userVaults[0].status.minted).to.equal(0);
expect(userVaults[0].status.maxMintable).to.equal(0);
expect(userVaults[0].status.totalCollateralValue).to.equal(0);
userVaults[0].status.collateral.forEach(asset => {
expect(asset.amount).to.equal(0);
});
expect(await Tether.balanceOf(protocol.address)).to.equal(protocolUSDTBalance.add(tetherValue));
expect(await protocol.getBalance()).to.equal(protocolETHBalance.add(ethValue));
});
});

Impact

In case of undercollateralized smartVault the owner could keep both the collateral and minted EUROs. But as it’s only in case of undercollateralization I consider it as a Medium.

Tools Used

Manual review

Recommendations

Instead of reverting the tx recalculate the final repayment could be paid during the execution of the liquidation. For example: repayment = minted > maxMintable() ? minted : maxMintable()

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

informational/invalid

Support

FAQs

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