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

Lack of balance check during burning EUROs

dentonylifer

Summary

When calling the "burn" function, the user must also pay the burning fee when burning EUROs. The protocol automatically calculates and transfers the fees to itself.

function burn(uint256 _amount) external ifMinted(_amount) {
uint256 fee = _amount * ISmartVaultManagerV3(manager).burnFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
//...
EUROs.burn(msg.sender, _amount);
IERC20(address(EUROs)).safeTransferFrom(msg.sender, ISmartVaultManagerV3(manager).protocol(), fee);
//...
}

But if the user wants to burn all the EUROs he has, the transaction will be reverted. This will happen because there is no check whether there are enough EUROs on the user's balance both for burning and for paying the fees.

POC

Replace this test with similar test in smartVault.js:

describe('burning', async () => {
it('User unable to burn all the EUROs he has', async () => {
const collateral = ethers.utils.parseEther('1');
await user.sendTransaction({to: Vault.address, value: collateral});
​
const burnedValue = ethers.utils.parseEther('100');
const mintedValue = ethers.utils.parseEther('100');
await Vault.connect(user).mint(user.address, mintedValue);
​
const mintingFee = mintedValue.mul(PROTOCOL_FEE_RATE).div(HUNDRED_PC);
const burningFee = burnedValue.mul(PROTOCOL_FEE_RATE).div(HUNDRED_PC);
​
await EUROs.connect(user).approve(Vault.address, burningFee);
burn = Vault.connect(user).burn(burnedValue);
await expect(burn).not.to.be.reverted;
await expect(burn).to.emit(Vault, 'EUROsBurned').withArgs(burnedValue, burningFee);
​
minted = (await Vault.status()).minted;
expect(minted).to.equal(mintedValue.add(mintingFee).sub(burnedValue));
​
const fees = mintingFee.add(burningFee);
expect(await EUROs.balanceOf(user.address)).to.equal(minted.sub(fees));
expect(await EUROs.balanceOf(protocol.address)).to.equal(fees);
});
});

This test will not pass, you will see this log: "AssertionError: Expected transaction NOT to be reverted, but it reverted with reason 'ERC20: transfer amount exceeds balance'"

Impact

User unable to burn all the EUROs he has.

Tools Used

Manual review, hardhat test.

Recommendations

Recommend to add checking whether there are enough funds on the user's balance to perform the transaction.

Updates

Lead Judging Commences

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

fee-loss

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

fee-loss

Support

FAQs

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