hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

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

Lack of Blacklist Check in User Whitelisting #107

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

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

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

Description: Description\ The whitelistUser function does not check if a user address is blacklisted before adding it to the whitelist. This oversight can lead to security issues by allowing blacklisted or malicious users to participate in the portfolio.

Attack Scenario\ ThewhitelistUser function whitelists users who are allowed to deposit in a particular portfolio. The function performs basic checks, such as ensuring the address is not zero and the number of users does not exceed the whitelist limit. However, it lacks a crucial check for blacklisted users. Here is the function:


/**
 * @notice This function whitelists users which can deposit in a particular portfolio
 * @param users Array of user addresses to be whitelisted by the asset manager
 */
function whitelistUser(
  address[] calldata users
) external virtual onlyWhitelistManager {
  uint256 len = users.length;
  if (len > protocolConfig.whitelistLimit())
    revert ErrorLibrary.InvalidWhitelistLimit();
  for (uint256 i; i < len; i++) {
    address _user = users[i];
    if (_user == address(0)) revert ErrorLibrary.InvalidAddress();
    whitelistedUsers[_user] = true;
  }
  emit UserWhitelisted(users);
}

Exploitation Scenario

An attacker identifies a user that is blacklisted but not checked during the whitelisting process. The attacker submits this blacklisted user to be added to the whitelist. The system adds the blacklisted user to the whitelist, allowing them to participate in the portfolio. The attacker or blacklisted user exploits the platform using their whitelisted status, causing potential harm. Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional) Recommendations To mitigate this vulnerability, implement a check for blacklisted users before adding them to the whitelist. Here is an example of how you can do this:

Add a Blacklist Check: Update the whitelistUser function to include a blacklist check. Maintain a Blacklist: Ensure there is a mechanism to manage and update a blacklist of user addresses.


// Mapping to track blacklisted users.
mapping(address => bool) public blacklistedUsers;

/**
 * @notice This function whitelists users which can deposit in a particular portfolio
 * @param users Array of user addresses to be whitelisted by the asset manager
 */
function whitelistUser(
  address[] calldata users
) external virtual onlyWhitelistManager {
  uint256 len = users.length;
  if (len > protocolConfig.whitelistLimit())
    revert ErrorLibrary.InvalidWhitelistLimit();
  for (uint256 i; i < len; i++) {
    address _user = users[i];
    if (_user == address(0) || blacklistedUsers[_user]) {
      revert ErrorLibrary.InvalidAddress();
    }
    whitelistedUsers[_user] = true;
  }
  emit UserWhitelisted(users);
}
langnavina97 commented 4 months ago

Blacklisting is not required. Asset managers will decide which users to add to the whitelist. @olaoyesalem