hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

Core smart contracts of Velvet Capital
Other
0 stars 1 forks source link

Potential Asset Manager Fee Bypass #98

Open olaoyesalem opened 3 months ago

olaoyesalem commented 3 months ago

Vulnerability Description:

The _chargeProtocolAndManagementFees function in smart contract systemically calculates and mints fees for asset management and protocol fees. However, a critical vulnerability exists where the asset manager may potentially receive zero fees due to specific conditions in the fee calculation process.

https://github.com/Velvet-Capital/velvet-core/blob/849629b1aacf32d84634d8c4ef1378527bce3bb3/contracts/fee/FeeCalculations.sol#L128

https://github.com/Velvet-Capital/velvet-core/blob/849629b1aacf32d84634d8c4ef1378527bce3bb3/contracts/fee/FeeModule.sol

Vulnerability Details:

In the _calculateProtocolAndManagementFeesToMint function, the calculation for managementFeeToMint relies on the calculation of feeReceiverShare, which is derived from streamingFees * ONE_ETH_IN_WEI / _totalSupply. If _totalSupply is significantly greater than streamingFees * ONE_ETH_IN_WEI, feeReceiverShare can round down to zero, resulting in managementFeeToMint being zero.

Consequently, when _mintProtocolAndManagementFees is called in _chargeProtocolAndManagementFees, assetManagerFeeToMint can be zero, allowing the asset manager fee to be bypassed .

Severity Level: High

Explanation: This vulnerability can lead to a complete bypass of fees intended for the asset manager, undermining the expected revenue generation mechanism for the protocol and management fees. The impact could significantly affect the financial sustainability and operational integrity of the contract.

Recommendations:

Implement Minimum Fee Requirements: Consider implementing minimum fee requirements to ensure that even if the calculated fee is low, there's a base amount that must be paid to the asset manager to prevent fee bypassing.

langnavina97 commented 3 months ago

We do not mint small values. If a deposit is so small that the minting fee would be zero or negligible, no fees will be minted. @olaoyesalem

olaoyesalem commented 3 months ago

/**
 * @dev Mints both protocol and management fees based on their respective calculated amounts.
 * @param _assetManagerFeeToMint The amount of asset manager fees to mint.
 * @param _protocolFeeToMint The amount of protocol fees to mint.
 */
function _mintProtocolAndManagementFees(
  uint256 _assetManagerFeeToMint,
  uint256 _protocolFeeToMint
) internal {
  _mintFees(
    assetManagementConfig.assetManagerTreasury(),
    _assetManagerFeeToMint
  );
  _mintFees(protocolConfig.velvetTreasury(), _protocolFeeToMint);
}

The _mintProtocolAndManagementFeesfunction mints both protocol and management fees based on the provided amounts. It calls the _mintFees function to handle the actual minting process.


/**
 * @dev Mints fees to a specified address if the amount is greater than the minimum fee threshold.
 * @param _to The address to mint the fees to.
 * @param _amount The amount of fees to mint.
 * @return The actual amount of fees minted.
 */
function _mintFees(address _to, uint256 _amount) internal returns (uint256) {
  if (_amount < MIN_MINT_FEE) return 0;

  portfolio.mintShares(_to, _amount);
  return _amount;
}

The _mintFees function checks if the amount is greater than the minimum fee threshold. If the amount is less than the threshold, it returns 0 without minting any fees. This behavior can lead to a successful transaction even when no fees are minted, effectively allowing an attacker to bypass the fee mechanism.

This vulnerability could allow an attacker to continuously execute transactions with amounts below the threshold, avoiding the minting of fees for the protocol and management. This can undermine the expected revenue generation for the protocol.

@langnavina97 @fonstack @

kakarottosama commented 3 months ago

@olaoyesalem your issues was submitted directly on github, not using Hats dapp.

According to rules, To participate, security researchers must submit their findings on-chain, and an automatic GitHub issue will be generated in the forked repository. , Submissions should be made using our Dapp, so, please submit it through dapp

olaoyesalem commented 3 months ago

Thanks

@olaoyesalem your issues was submitted directly on github, not using Hats dapp.

According to rules, To participate, security researchers must submit their findings on-chain, and an automatic GitHub issue will be generated in the forked repository. , Submissions should be made using our Dapp, so, please submit it through dapp

Thanks I have done that.