hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

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

Inconsistent Fee Minting Logic #104

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: @olaoyesalem Twitter username: salthegeek1 Submission hash (on-chain): 0x06cb50e8b225f95a4fb970af72e465348e897fe64bd972a364f96c0fa3c7a41c Severity: high

Description: Description\ The codebase contains two implementations of the _mintFees function, each with different logic for handling fees below the minimum threshold (MIN_MINT_FEE). This inconsistency can be exploited by attackers to bypass the fee mechanism, undermining the expected revenue generation for the protocol and management.

Attack Scenario\ Implementation 1

/**
 * @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) {
    portfolio.mintShares(_to, _amount);
    return _amount;
  }
  return 0;
}

Implementation 2

/**
 * @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 vulnerability arises from the difference in the conditions used to check the minimum fee threshold. The first implementation mints fees only if _amount is greater than or equal to MIN_MINT_FEE, while the second implementation returns 0 if _amount is less than MIN_MINT_FEE, and otherwise mints the fees.

An attacker can exploit this inconsistency by ensuring that the fee amount is always below the minimum threshold. As a result:

Fees will not be minted, leading to a bypass of the fee mechanism.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional) To mitigate this vulnerability, ensure that the _mintFees function is consistently implemented across the codebase. Adopt a single, clear logic for handling fee amounts:

    
    /**
    * @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) {
    portfolio.mintShares(_to, _amount);
    return _amount;
    }
    return 0;
    }
olaoyesalem commented 3 months ago

Reasons why it was flagged invalid? @langnavina97

langnavina97 commented 3 months ago

The correct implementation is in the FeeModule contract, the other code snippet is from the mock folder which is out of scope. @olaoyesalem