Summary
User can't withdraw max collateral from vault if have any minted euros because wrong calculations in canRemoveCollateral function
Vulnerability Details
A user should withdraw all the collateral, except enough collateral require to back the minted euros, but with current implementation its not possible because canRemoveCollateral function is using maxMintable in calculation instead of euroCollateral
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;
}
How this will work:-
User deposited 6000 euro worth of collateral in the vault
maxMintable at 120% collateral rate will be 5000 euro ie:- ((6000 * 100000)/120000) = 5000
function maxMintable() private view returns (uint256) {
return euroCollateral() * ISmartVaultManagerV3(manager).HUNDRED_PC() / ISmartVaultManagerV3(manager).collateralRate();
}
User minted 2000 euros
Total collateral required to back 2000 minted euros is 2400 euro worth collateral ie:- ( mintedEuros * collateralRate)/ HUNDRED_PC, (2000 * 120000)/100000
Available collateral to withdraw is 6000 - 2400 = 3600 euro worth collateral, but with current implementation user can only withdraw 3000 worth of collateral, which is 600 euro less worth of collateral
This 600 can go up depending upon the size of collateral and minted euros
Here is the POC( run this test in smartVault.js test file )
it("user can not withdraw max collateral", 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);
const value = 6360000000;
await Tether.mint(Vault.address, value);
const mintvalue = ethers.utils.parseEther("1990");
await Vault.connect(user).mint(user.address, mintvalue);
let { minted, maxMintable, totalCollateralValue } = await Vault.status();
console.log(totalCollateralValue);
console.log(maxMintable);
console.log(minted);
const removeValue = 3816000000;
let removeCollateral = Vault.connect(user).removeCollateral(
USDTBytes,
removeValue,
user.address
);
await expect(removeCollateral).to.be.revertedWith("err-under-coll");
});
Impact
No loss of funds but will grief user
Tools Used
Manual Review
Recommendations
Use euroCollateral instead of maxMintable in calculation
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;
}
function canRemoveCollateral(ITokenManager.Token memory _token, uint256 _amount) private view returns (bool) {
if (minted == 0) return true;
+ uint256 currentEuroCollateral = euroCollateral();
uint256 eurValueToRemove = calculator.tokenToEurAvg(_token, _amount);
+ uint256 requiredCollateralValue =
+ minted * ISmartVaultManagerV3(manager).collateralRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
+ return currentEuroCollateral >= eurValueToRemove
+ && requiredCollateralValue <= currentEuroCollateral - eurValueToRemove;
}
