hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

Add function to remove whitelisted NFTs #60

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

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

Github username: @rilwan99 Twitter username: Ril11111 Submission hash (on-chain): 0x5d236dc878c81b5201a3e978ea31fcd16d8b2b4fe78166791913b79ea616b82d Severity: low

Description: Description\ In ManagedNFTManagerUpgradeable.sol, the Managed NFT admin can only add NFTs to the whitelist array. However, it is missing the functionality to remove whitelisted NFTs. If any whitelisted token NFT has an issue, it cannot be removed from the list.

Attack Scenario\ WhiteListed NFTs have voting privileges, such as being able to vote during the distribution window. This can be found in VoterUpgradeableV1_2.sol.

function vote(uint256 _tokenId, address[] calldata _poolVote, uint256[] calldata _weights) external nonReentrant {
    _voteDelay(_tokenId);

    require(IVotingEscrow(_ve).isApprovedOrOwner(msg.sender, _tokenId), "!approved/Owner");
    require(_poolVote.length == _weights.length, "Pool/Weights length !=");

    IManagedNFTManager managedNFTManagerCache = IManagedNFTManager(managedNFTManager);
    require(!managedNFTManagerCache.isDisabledNFT(_tokenId), "disabled managed nft");

    if (!managedNFTManagerCache.isWhitelistedNFT(_tokenId)) {
        _checkEndVoteWindow();
    }

    _vote(_tokenId, _poolVote, _weights);

    lastVoted[_tokenId] = _epochTimestamp() + 1;
}

function _checkEndVoteWindow() internal view {
    require(block.timestamp < (block.timestamp - (block.timestamp % _WEEK) + _WEEK - distributionWindowDuration), "distribute window");
}

The impact of this vulnerability is the following:

  1. Permanent Privileged Access: Whitelisted NFTs have special voting privileges, including the ability to vote during the distribution window when regular NFTs cannot. If a whitelisted NFT becomes compromised or its owner becomes malicious, there's no way to revoke these privileges.
  2. Manipulation of Voting Process: Since whitelisted NFTs can vote during the distribution window, a compromised or malicious whitelisted NFT could potentially manipulate voting outcomes by casting last-minute votes when other participants can't react.
  3. Inability to Respond to Security Threats: If a vulnerability is discovered in a specific whitelisted NFT, or if its private key is compromised, the protocol has no mechanism to quickly remove its privileged status, leaving the system exposed.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)\ The following function can be added to remove whitelised nfts.

function updateWhitelistedNFTStatus(uint256 tokenId_, bool isWhitelisted_) external onlyRole(MANAGED_NFT_ADMIN) {
    isWhitelistedNFT[tokenId_] = isWhitelisted_;
    emit SetWhitelistedNFT(tokenId_, isWhitelisted_);
}
0xmahdirostami commented 3 months ago

There already is a function which sets the boolean for a token id which admin can switch the whitelist on and off