hats-finance / StakeWise-0xd91cd6ed6c9a112fdc112b1a3c66e47697f522cd

Liquid staking protocol for Ethereum
Other
0 stars 0 forks source link

VaultWhitelist::_updateWhitelist(), _setWhitelister(), __VaultWhitelist_init() - no input validation checks for parameters `account` and `_whitelister`, for address(0) value. Since a mistake can be corrected, this is LOW/MEDIUM. #140

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

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

Github username: @dappconsulting Submission hash (on-chain): 0xaee6b926d93428f533f8bd8d5d16925fb6aab3b53f0cda456796fcf43a723cad Severity: low

Description: Description\

VaultWhitelist::_updateWhitelist(), _setWhitelister(), __VaultWhitelist_init() - no input validation checks for parameters account and _whitelister, for address(0) value. Since a mistake can be corrected, this is LOW/MEDIUM.

https://github.com/stakewise/v3-core/blob/dafcac34b947bc4763c89d9e5eeb35d1849bdc22/contracts/vaults/modules/VaultWhitelist.sol#L41 https://github.com/stakewise/v3-core/blob/dafcac34b947bc4763c89d9e5eeb35d1849bdc22/contracts/vaults/modules/VaultWhitelist.sol#L51 https://github.com/stakewise/v3-core/blob/dafcac34b947bc4763c89d9e5eeb35d1849bdc22/contracts/vaults/modules/VaultWhitelist.sol#L61

Recommendation:

Add the following zero address check to the functions before the values of the address parameters are assigned/used in the functions:

_updateWhitelist():

if (account == address(0)) revert Errors.ZeroAddress();

_setWhitelister():

if (_whitelister == address(0)) revert Errors.ZeroAddress();

__VaultWhitelist_init():

if (_whitelister == address(0)) revert Errors.ZeroAddress();

Attack Scenario\ No attack, unless admin/owner cant be trusted, goes rogue. DoS of whitelister & whitelisting functionality.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

tsudmi commented 1 year ago

The whitelister can be set to zero address by admin to disable the whitelisting functionality. This could be used to indicate that the whitelist is now closed. As of the zero address check in _updateWhitelist it only adds to the gas cost as there is no way to deposit from the zero address, so the whitelist check will never happen for such address.