hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

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

UserWhitelistManagement wrong check, open for exceeding `whitelistLimit` #32

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

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

Description: Description:

In UserWhitelistManagement, the check whitelistLimit is not correct, as it's checking the users parameter, not the already-whitelisted users.

The check should be counting the whitelistedUsers which is already true.

File: UserWhitelistManagement.sol
29:   /**
30:    * @notice This function whitelists users which can deposit in a particular portfolio
31:    * @param users Array of user addresses to be whitelisted by the asset manager
32:    */
33:   function whitelistUser(
34:     address[] calldata users
35:   ) external virtual onlyWhitelistManager {
36:     uint256 len = users.length;
37:     if (len > protocolConfig.whitelistLimit())
38:       revert ErrorLibrary.InvalidWhitelistLimit();
39:     for (uint256 i; i < len; i++) {
40:       address _user = users[i];
41:       if (_user == address(0)) revert ErrorLibrary.InvalidAddress();
42:       whitelistedUsers[_user] = true;
43:     }
44:     emit UserWhitelisted(users);
45:   }

Impact:

whitelistedUsers can exceed the whitelistLimit

Mitigation:

Add a whitelistedUsers counter, and compare it with whitelistLimit

langnavina97 commented 1 week ago

The whitelist limit is intended to restrict the number of users that can be whitelisted in a single transaction, not the total number of whitelisted users. This prevents denial-of-service (DoS) attacks by ensuring that the operation remains manageable. The protocol configuration allows us to adjust this limit as needed for security and efficiency.