The Standard

DeFiHardhat

20,000 USDC

View results

Previous Next

Submission Details

Severity: low

Invalid

Inability to Burn Initial Minted Amount, minting fee not charged when taking a loan and Counter Intuitive Redeem Collateral leading to lock funds

0xrektified

Summary

Those issues are reported as one issue as the overall Vault usage is impacting the User directly and the complete borrowing flow outline it

1 -The burn function in the smart contract exhibits a flaw where it reverts if a user attempts to burn the exact amount of EUROs previously minted.
This leads to a situation where users must acquire additional EUROs beyond their initial minted amount on a DEX to burn the original minted amount.

Based on documentations the amount should be charge to the user when burning and the user should NOT be in charge to calculate himself the exact amount of the burning fees that should be left in his wallet to pay for the fees.

Burning Fee: This fee is charged when people repay their debt. It serves to fine-tune stability

and generate income for research, development, business expansion, and TST staking

rewards.

2 - Regarding the minting fee, it is not charged when minting the asset but when burning the minted asset which, leaving the protocol with potential unpaid fees if the vault is liquidated

Minting Fee: This one-time fee is charged when borrowers take out a loan.

3 - To redeem their collateral users should make a complicated calculation:

How much extra EUROs they should purchase and hold in their wallet for the burning fee to be charged (as the contract use a safeTransferFrom)
How much EUROs should be burn (counter intuitive as the amount provided to the burn method is different from the amount that will be charge see POC2)
It is in fact the original amount + minting fee to be able to reached minted == 0 checked in the canRemoveCollateral method
How much the user should approve so the effective burn fee can be charge to the user.

The issue is classified as HIGH severity due to its significant impact on users' ability to retrieve their collateral and the potential for fund lockup.

Vulnerability Details

To be able to run the test in this finding follow the following recommendation

INITIAL FOUNDRY SETUP FOR POC

Follow the following tutorial to install foundry in your local repo

https://hardhat.org/hardhat-runner/docs/advanced/hardhat-and-foundry

When hardhat-foundry is installed, rename the current test folder to hardhat_test so forge won't look at it.

Create/Update the foundry.toml fil with the current setup, to allow compilation with viaIR

[profile.default]

src = 'contracts'

out = 'out'

libs = ['node_modules', 'lib']

test = 'test'

cache_path = 'cache_forge'

viaIR= true

Create a new test folder, with the following file test contract in it, this contract deploy the whole contract stack.

The smartVaultManager is deployed with the proxy.
NFTMetadataGenerator is replaced by a Dummy address to lower the compilation time.

2 helpers are used in all the pocs:

_deployVault: Used to deploy the user Vault and retrieve the vault address
_send: Is used to send ETH

// SPDX-License-Identifier: UNLICENSED

pragma solidity ^0.8.17;

import "forge-std/Test.sol";

import "./utils/ERC20Mock.sol";

import "./utils/EUROsMock.sol";

import "./utils/ChainlinkMock.sol";

import "./utils/TokenManagerMock.sol";

import "./utils/SmartVaultDeployerV3.sol";

import "./utils/SwapRouterMock.sol";

import "./utils/SmartVaultIndex.sol";

import "../contracts/LiquidationPoolManager.sol";

import "./utils/SmartVaultManager.sol";

import "../contracts/SmartVaultManagerV5.sol";

import "openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol";

import "openzeppelin-contracts/contracts/token/ERC721/IERC721Receiver.sol";

import {Test} from "forge-std/Test.sol";

import {StdInvariant} from "forge-std/StdInvariant.sol";

import {console2} from "forge-std/console2.sol";

import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";

contract ImplDeployerProxy is ERC1967Proxy {

constructor(address impl, bytes memory data) ERC1967Proxy(impl, data) {}

function upgradeTo(address newImplementation) public {

_upgradeTo(newImplementation);

}

contract ContractBTest is Test {

uint256 baseTime = 1710000000;

uint256 DEFAULT_COLLATERAL_RATE = 120000; // 120%

uint256 PROTOCOL_FEE_RATE = 500; // 0.5%

uint32 POOL_FEE_PERCENTAGE = 50000; // 50%

bytes32 immutable WETH = "ETH";

address WETH_ADDRESS = 0x82aF49447D8a07e3bd95BD0d56f35241523fBab1;

address WBTC_ADDRESS = 0x2f2a2543B76A4166549F7aaB2e75Bef0aefC5B0f;

EUROsMock EUROs;

ERC20Mock TST;

ERC20Mock WBTC;

ChainlinkMock ClEthUsd;

ChainlinkMock ClBtcUsd;

ChainlinkMock ClEurUsd;

SmartVaultManager smartVaultManagerV1;

ImplDeployerProxy proxy;

SmartVaultManagerV5 smartVaultManagerV5;

SmartVaultIndex smartVaultIndex;

SmartVaultV3 smartVault;

LiquidationPoolManager liquidationPoolManager;

LiquidationPool liquidationPool;

TokenManagerMock tokenManagerMock;

address user = makeAddr("user");

address deployer = makeAddr("deployer");

address protocol = makeAddr("protocol");

address userLiquidator = makeAddr("userLiquidator");

address maliciousUser = makeAddr("maliciousUser");

address NFTMetadataGenerator = makeAddr("Dum");

address liquidator = makeAddr("liquidator");

function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) {

return IERC721Receiver.onERC721Received.selector;

}

function setUp() public {

vm.startPrank(deployer);

address implementationV1 = address(smartVaultManagerV1);

WBTC = new ERC20Mock('Wrapped BTC', 'WBTC', 8);

// Set chainlink price Mock to 2200$

ClEthUsd = new ChainlinkMock('ETH / USD');

ClBtcUsd = new ChainlinkMock('BTC / USD');

ClEurUsd = new ChainlinkMock('EUR / USD');

vm.warp(baseTime);

ClEthUsd.setPrice(220000000000); // 2200$

vm.warp(baseTime);

ClBtcUsd.setPrice(4400000000000); // 44000$

vm.warp(baseTime);

ClEurUsd.setPrice(110000000); // 1.10$

EUROs = new EUROsMock();

tokenManagerMock = new TokenManagerMock(WETH, address(ClEthUsd));

tokenManagerMock.addAcceptedToken(address(WBTC), address(ClBtcUsd));

address smartVaultDeployerV3 = address(new SmartVaultDeployerV3(WETH, address(ClEurUsd)));

smartVaultIndex = new SmartVaultIndex();

address swapRouterMock = address(new SwapRouterMock());

// Make sure everything is deployed correctly

bytes memory data = abi.encodeWithSignature(

"initialize(uint256,uint256,address,address,address,address,address,address,address)",

DEFAULT_COLLATERAL_RATE,

PROTOCOL_FEE_RATE,

address(EUROs),

protocol,

liquidator,

address(tokenManagerMock),

smartVaultDeployerV3,

address(smartVaultIndex),

NFTMetadataGenerator

);

// Set contract and proxy

proxy = new ImplDeployerProxy(address(new SmartVaultManager()), data);

proxy.upgradeTo(address(new SmartVaultManagerV5()));

smartVaultManagerV5 = SmartVaultManagerV5(address(proxy));

// Settup vault manager

smartVaultIndex.setVaultManager(address(smartVaultManagerV5));

bytes32 adminRole = EUROs.DEFAULT_ADMIN_ROLE();

EUROs.grantRole(adminRole, address(smartVaultManagerV5));

// For testing purpose we allow this contract to mint EUROs

EUROs.grantRole(adminRole, address(this));

EUROs.grantRole(EUROs.MINTER_ROLE(), address(this));

// Liquidation setup

TST = new ERC20Mock('The Standard Token', 'TST', 18);

liquidationPoolManager =

new LiquidationPoolManager(address(TST), address(EUROs), address(smartVaultManagerV5), address(ClEurUsd), payable(protocol), POOL_FEE_PERCENTAGE);

liquidationPool = LiquidationPool(liquidationPoolManager.pool());

smartVaultManagerV5.setLiquidatorAddress(address(liquidationPoolManager));

smartVaultManagerV5.setProtocolAddress(address(liquidationPoolManager));

EUROs.grantRole(EUROs.BURNER_ROLE(), address(liquidationPool));

vm.stopPrank();

}

function _send(address target, uint256 amount) private {

(bool ok,) = address(target).call{value: amount}("");

require(ok, "Send ETH fail");

}

function _deployVault() public returns (address payable vaultAddress) {

vm.recordLogs();

//Mint vault through smartVaultManager and extract address

smartVaultManagerV5.mint();

Vm.Log[] memory logs = vm.getRecordedLogs();

bytes32 vaultAddressBytes = logs[4].topics[1];

vaultAddress = payable(address(uint160(uint256(vaultAddressBytes))));

smartVault = SmartVaultV3(vaultAddress);

}

function testSetup() public {

startHoax(user, 100 ether);

address vaultAddress = _deployVault();

_send(address(smartVault), 1 ether); // User send 1 ETH to the contract

smartVault.mint(user, 100 ether);

vm.stopPrank();

assert(EUROs.balanceOf(user) == 100 ether);

}

The core of the vulnerability lies in the mint and burn function's handling of fee deductions.

The mint method is adding the fee to the minted variable amount so the user will have to later cover for this fee (while burning).

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;

EUROs.mint(_to, _amount);

EUROs.mint(ISmartVaultManagerV3(manager).protocol(), fee);

emit EUROsMinted(_to, _amount, fee);

}

When a user burns EURO tokens, a fee is calculated and expected to be transferred from the user's account to a protocol address.

function burn(uint256 _amount) external ifMinted(_amount) {

-> uint256 fee = _amount * ISmartVaultManagerV3(manager).burnFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();

minted = minted - _amount;

EUROs.burn(msg.sender, _amount);

-> IERC20(address(EUROs)).safeTransferFrom(msg.sender, ISmartVaultManagerV3(manager).protocol(), fee);

emit EUROsBurned(_amount, fee);

}

However, if a user attempts to burn an amount equal to their entire EURO balance, they fall short of covering the total (amount + fee), resulting in the safeTransferFrom function reverting.

Even if the user should provide the fees in top his initial minted amount to claim a reward he shouldn't be blocked when trying to burn EUROs without any intention to claim his rewards

See the POC bellow

POC 1 revert

function _send(address target, uint256 amount) private {

(bool ok,) = address(target).call{value: amount}("");

require(ok, "Send ETH fail");

}

address user = makeAddr("user");

function testUserMintAndBurnEuro() public {

// we add 100 ETH to the user balance

startHoax(user, 100 ether);

vm.recordLogs();

//Mint vault through smartVaultManager and extract address

smartVaultManagerV5.mint();

Vm.Log[] memory logs = vm.getRecordedLogs();

bytes32 vaultAddressBytes = logs[4].topics[1];

address payable vaultAddress = payable(address(uint160(uint256(vaultAddressBytes))));

smartVault = SmartVaultV3(vaultAddress);

//Sent 1 ETH as collateral to the vault

_send(address(smartVault), 1 ether);

// Mint 100 EUROs

smartVault.mint(user, 100 ether);

assert(EUROs.balanceOf(user) == 100 ether); // Balance 100 EUROs

assert(EUROs.balanceOf(protocol) == 0.5 ether); // Balance 0.5 EUROs

EUROs.approve(address(smartVault), 100 ether);

// Burn 100 EUROs FAIL

// [FAIL. Reason: revert: ERC20: transfer amount exceeds balance]

smartVault.burn(100 ether);

vm.stopPrank();

}

The above PoC demonstrated that after minting 100 EUROs, a user cannot burn this entire amount due to the fee deduction, leading to a situation where the user must acquire additional EUROs in his wallet (more than they initially minted) to cover the burn fee and successfully redeem their collateral. Even if the user is clearly over collateralised. See POC2 for more details

This requirement for extra EUROs to match the fee added to the minted variable and to cover the fee sent to the protocol during burning is counterintuitive and goes against the expected functionality of the contract as demonstrated bellow.

POC 2

function testUserMintAndBurnEuroRemoveCollateral() public {

// We mint an extra 10.025 EURO that we will use later to match the invalid calculation

EUROs.mint(address(this), 10.025 ether);

// Start prank and credit 100 ETH to the user

startHoax(user, 100 ether);

vm.recordLogs();

//Mint vault through smartVaultManager and extract address

smartVaultManagerV5.mint();

Vm.Log[] memory logs = vm.getRecordedLogs();

bytes32 vaultAddressBytes = logs[4].topics[1];

address payable vaultAddress = payable(address(uint160(uint256(vaultAddressBytes))));

smartVault = SmartVaultV3(vaultAddress);

console2.log(vaultAddress);

//Send 1 ETH as collateral to allow the user to mint EUROs

_send(address(smartVault), 1 ether);

assert(user.balance == 99 ether);

// Mint 1000 EUROs

smartVault.mint(user, 1000 ether);

vm.stopPrank();

* Send 10.025 extra EUROs from the testing contract to the user which represent the

* real user flow of a user purchasing extra token from an exchange to be able to unlock his collateral

* 5 EUROs to match the fee while minting as it was added to the `minted` variable

* so minted can be zero when removing collateral

* 5.025 EUROs to match the fee sent to the protocol using safeTransferFrom while burning

vm.prank(address(this));

EUROs.transfer(user, 10.025 ether);

assert(EUROs.balanceOf(user) == 1010.025 ether);

assert(EUROs.balanceOf(protocol) == 5 ether);

vm.startPrank(user);

// Allow the contract to transfer the 1000 to burn + 5.025 that the contract will transfer to the protocol while burning

EUROs.approve(address(smartVault), 1005.025 ether);

// the user has to require to Burn 1005 EUROs to be able to free his collateral

smartVault.burn(1005 ether);

// the user can only successfully remove his collateral after finding the correct values

smartVault.removeCollateralNative(1 ether, payable(user));

assert(user.balance == 100 ether);

vm.stopPrank();

}

As the contract doesn't provide any helper to calculate the exact amount to send / approve and the extra token amount required to be able to remove the collateral it's likely that most users will have an hard time to be able to remove their collateral.

Additionally the current UI doesn't handle the burning fees

Impact

User Fund Lockup:

Users won't be able to burn the amount of EURO that they just minted, instead of just deducting the fee from the minted amount.

Users are unable to redeem their full collateral if they are not able to proceed to complicated exact calculation, leading to a lockup of funds in the contract. This can be particularly problematic in scenarios where obtaining additional EUROs to cover the fee is challenging or costly, as they could pay a premium to acquire EURO on a DEX when they decide to repay the loan

Operational Disruption: This issue disrupts the normal operation of the contract, forcing users to engage in unconventional methods to retrieve their collateral.

Tools Used

Forge Proof of Concept
Manual Code Review: Analysis of the contract code to understand the logic and flow that leads to the vulnerability.

Recommendations

For the mint Function:

Deduct Fee from Minted Amount: The mint function should deduct the minting fee from the amount that is to be minted for the user. This means the actual amount minted for the user is _amount - fee.

Ensure Collateral Backing: The total amount minted (after fee deduction) should be fully backed by the user's collateral. The minted variable should reflect only the net amount minted for the user.

For the burn Function:

Adjust Burn Amount: Modify the burn function to burn only the net amount from the user’s balance, which is _amount - fee. This allows the user to redeem the full collateral without needing additional EUROs.

Handle Fee Appropriately: Instead of transferring the fee to the protocol, adjust the contract's logic to ensure that the fee amount is either:
Deducted from the user's collateral during the redemption process, or
Accumulated within the contract in a way that doesn't affect the collateral backing of the remaining EUROs.

Ensuring Full Collateralization:
Collateralization Integrity: It's critical to ensure that at all times, the total amount of EUROs (both in circulation and as fees) is fully backed by an equivalent amount of collateral. This might involve intricate adjustments in how fees are accounted for within the contract's balance and collateral calculations.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago

Submission Judgement Published

Invalidated

Reason: Non-acceptable severity

Assigned finding tags:

informational/invalid

Prize pool breakdown

Total prize

20,000 USDC

nSLOC

609

33 USDC / LOC

High Medium

17,500 USDC

Low

2,500 USDC

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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