hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

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

Lack of Blacklist Check in Token Whitelisting #106

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

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

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

Description: Description\ The TokenWhitelistManagement contract does not check if a token address is blacklisted before adding it to the whitelist. This oversight can lead to security and compliance issues by allowing blacklisted or malicious tokens to participate in the platform.

Attack Scenario\ The TokenWhitelistManagement contract manages the whitelisting of tokens, ensuring that only authorized tokens are used within the system. The contract initializes with a list of whitelisted tokens and sets the flag for token whitelisting. The relevant functions are as follows:


// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.17;

import {ErrorLibrary} from "../../library/ErrorLibrary.sol";
import {AssetManagerCheck} from "./AssetManagerCheck.sol";
import {IProtocolConfig} from "../../config/protocol/IProtocolConfig.sol";
import {Initializable} from "@openzeppelin/contracts-upgradeable-4.9.6/proxy/utils/Initializable.sol";

/**
 * @title TokenWhitelistManagement
 * @dev Manages the whitelisting of tokens, determining which tokens are authorized for use within the system.
 * This enables control over the tokens that can participate in the platform, enhancing security and compliance.
 */
abstract contract TokenWhitelistManagement is AssetManagerCheck, Initializable {
  // Reference to the protocol configuration contract
  IProtocolConfig private protocolConfig;

  // Mapping to track whitelisted tokens.
  mapping(address => bool) public whitelistedTokens;

  // Flag to indicate if token whitelisting is enabled.
  bool public tokenWhitelistingEnabled;

  // Events for logging changes to the whitelist.
  event TokenWhitelisted(address[] indexed tokens);
  event TokensRemovedFromWhitelist(address[] indexed tokens);

  /**
   * @dev Initializes the contract with a list of tokens to be whitelisted and sets the whitelisting enabled flag.
   * @param _whitelistTokens Initial list of tokens to whitelist.
   * @param _tokenWhitelistingEnabled Flag indicating if token whitelisting is enabled.
   */
  function __TokenWhitelistManagement_init(
    address[] calldata _whitelistTokens,
    bool _tokenWhitelistingEnabled,
    address _protocolConfig
  ) internal onlyInitializing {
    if (_protocolConfig == address(0)) revert ErrorLibrary.InvalidAddress();
    protocolConfig = IProtocolConfig(_protocolConfig);
    tokenWhitelistingEnabled = _tokenWhitelistingEnabled;
    if (tokenWhitelistingEnabled) {
      _addTokensToWhitelist(_whitelistTokens);
    }
  }

  /**
   * @dev Internal function to add tokens to the whitelist.
   * @param _tokens Array of token addresses to whitelist.
   */
  function _addTokensToWhitelist(address[] calldata _tokens) internal {
    uint256 tokensLength = _tokens.length;
    if (tokensLength > protocolConfig.whitelistLimit())
      revert ErrorLibrary.InvalidWhitelistLimit();
    for (uint256 i; i < tokensLength; i++) {
      address _token = _tokens[i];
      if (_token == address(0)) {
        revert ErrorLibrary.InvalidAddress();
      }
      whitelistedTokens[_token] = true;
    }
    emit TokenWhitelisted(_tokens);
  }
}

Operational Risk: Malicious or blacklisted tokens could disrupt the normal operations of the platform, affecting the user experience and trust in the system.

Exploitation Scenario

An attacker identifies a token that is blacklisted but not checked during the whitelisting process. The attacker submits this blacklisted token to be added to the whitelist. The system adds the blacklisted token to the whitelist, allowing it to participate in the platform. The attacker exploits the platform using the blacklisted token, causing potential harm. Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional) To mitigate this vulnerability, implement a check for blacklisted tokens before adding them to the whitelist. Here's an example of how you can do this: Add a Blacklist Check: Update the_addTokensToWhitelistfunction to include a blacklist check. Maintain a Blacklist: Ensure there is a mechanism to manage and update a blacklist of token addresses.

// Mapping to track blacklisted tokens.
mapping(address => bool) public blacklistedTokens;

/**
 * @dev Internal function to add tokens to the whitelist.
 * @param _tokens Array of token addresses to whitelist.
 */
function _addTokensToWhitelist(address[] calldata _tokens) internal {
  uint256 tokensLength = _tokens.length;
  if (tokensLength > protocolConfig.whitelistLimit())
    revert ErrorLibrary.InvalidWhitelistLimit();
  for (uint256 i; i < tokensLength; i++) {
    address _token = _tokens[i];
    if (_token == address(0) || blacklistedTokens[_token]) {
      revert ErrorLibrary.InvalidAddress();
    }
    whitelistedTokens[_token] = true;
  }
  emit TokenWhitelisted(_tokens);
}
langnavina97 commented 2 months ago

We're leaving it to the asset managers to decide which tokens, if any, they want to add to the whitelist. This allows users to make an informed decision about depositing into the portfolio, knowing whether there are any whitelisted tokens or if the asset manager could potentially rebalance to any token. Blacklisting is not required from our side. @olaoyesalem