DOS on chargePerformanceFee if ANY token is disabled

Github username: -- Twitter username: rnemes4 Submission hash (on-chain): 0x8fc6a02dec5a418f09c53ddd63e5147f727202bfedb62adad7ce944545484bf6 Severity: low

Description: Description\ By seting a portfolio token to disabled a protocol owner will prevent the asset manager from minting any performance fees due to getVaultValueInUSD reverting in contracts/core/calculations/VaultCalculations.sol

The following is a description of the call path, showing how this affects the chargePerformanceFee call in contracts/fee/FeeModule.sol

A token in the vault can be disabled by the protocolOwner

contracts/config/protocol/TokenManagement.sol

  /**
   * @notice Disables a token from interaction on the platform. Can only be called by the protocol owner.
   * @param _token Address of the token to be disabled.
   */
  function disableToken(address _token) external onlyProtocolOwner {
    if (_token == address(0)) revert ErrorLibrary.InvalidAddress();
    isEnabled[_token] = false;
    emit TokenDisabled(_token);
  }

In the feeModule we have the chargePerformanceFee function which can be called by the assetOwner

contracts/fee/FeeModule.sol

  /**
   * @dev Calculates and mints performance fees based on the vault's performance relative to a high watermark.
   */
  function chargePerformanceFee() external onlyAssetManager nonReentrant {
    uint256 totalSupply = portfolio.totalSupply();

    uint256 vaultBalance = portfolio.getVaultValueInUSD(
      IPriceOracle(protocolConfig.oracle()),
      portfolio.getTokens(),
      totalSupply,
      portfolio.vault()
    ); // @audit will revert if ANY token is disabled

This function calls into portfolio.getVaultValueInUSD which will revert with ErrorLibrary.TokenNotEnabled() if any token has been dissabled.

contracts/core/calculations/VaultCalculations.sol

function getVaultValueInUSD(
    IPriceOracle _oracle,
    address[] memory _tokens,
    uint256 _totalSupply,
    address _vault
  ) external view returns (uint256 vaultValue) {
    if (_totalSupply == 0) return 0;

    uint256 _tokenBalanceInUSD;
    uint256 tokensLength = _tokens.length;
    for (uint256 i; i < tokensLength; i++) {
      address _token = _tokens[i];
      if (!protocolConfig().isTokenEnabled(_token))
        revert ErrorLibrary.TokenNotEnabled();
      _tokenBalanceInUSD = _oracle.convertToUSD18Decimals(
        _token,
        IERC20Upgradeable(_token).balanceOf(_vault)
      );

      vaultValue += _tokenBalanceInUSD;
    }
  }

Attack Scenario\ This is not an attack but can cause a DOS of charging perfomance fees. This is the only place in the protocol that uses the disabled token feature and is possibly an oversight from a previous audit https://github.com/Velvet-Capital/audits/blob/main/Velvet_Capital_V2_Security_Audit_Report.pdf issue SHB.21.1 where a similar issue was reported and the enabled chack was removed.

Attachments

Proof of Concept (PoC) File https://gist.github.com/Jonsey/04717dbabc94e7c7d29d1ab36148d956
Revised Code File (Optional) It is recomended to just carry on when a token is disabled or maybe consider removing the disabled functionality completely.

function getVaultValueInUSD(
    IPriceOracle _oracle,
    address[] memory _tokens,
    uint256 _totalSupply,
    address _vault
  ) external view returns (uint256 vaultValue) {
    if (_totalSupply == 0) return 0;

    uint256 _tokenBalanceInUSD;
    uint256 tokensLength = _tokens.length;
    for (uint256 i; i < tokensLength; i++) {
      address _token = _tokens[i];
      if (!protocolConfig().isTokenEnabled(_token))
        continue;
      _tokenBalanceInUSD = _oracle.convertToUSD18Decimals(
        _token,
        IERC20Upgradeable(_token).balanceOf(_vault)
      );

      vaultValue += _tokenBalanceInUSD;
    }
  }

hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77