Lack of `Disabled NFT` Check in `onDettach` Function

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x956643094f469c987a8d6d5418851936248ee3a723121fc8deeade8e1d33fce8 Severity: medium

Description: Description\ The onDettach function in the CompoundVeFNXManagedNFTStrategyUpgradeable contract does not check whether the NFT is disabled before proceeding with the detachment process. This can lead to unintended behavior or errors if operations are performed on NFTs that are under maintenance or have been intentionally disabled. Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

1. Proof of Concept (PoC) File

   The onDettach function in CompoundVeFNXManagedNFTStrategyUpgradeable.sol:

/**
 * @notice Detaches an NFT from the strategy, withdrawing all associated rewards and balances.
 * @dev Handles the process of detaching an NFT, ensuring all accrued benefits are properly managed and withdrawn.
 *
 * @param tokenId_ The identifier of the NFT to detach.
 * @param userBalance_ The remaining balance or stake associated with the NFT at the time of detachment.
 * @return lockedRewards The amount of rewards locked and harvested upon detachment.
 */
function onDettach(uint256 tokenId_, uint256 userBalance_) external override onlyManagedNFTManager returns (uint256 lockedRewards) {
    ISingelTokenVirtualRewarder virtualRewarderCache = ISingelTokenVirtualRewarder(virtualRewarder);
    virtualRewarderCache.withdraw(tokenId_, userBalance_);
    lockedRewards = virtualRewarderCache.harvest(tokenId_);
    emit OnDettach(tokenId_, userBalance_, lockedRewards);
}

The onDettach function does not verify if the tokenId_ is disabled. This can lead to operations being performed on NFTs that are under maintenance or have been intentionally disabled, potentially causing unintended behaviors.

2. Revised Code File (Optional)
   • Introduce a check in the onDettach function to ensure the tokenId_ is not disabled before proceeding with the detachment process. This can be done by using the isDisabledNFT function from the ManagedNFTManagerUpgradeable contract.

require(!isDisabledNFT(tokenId_), "Token ID is disabled");

[0xmahdirostami]

The dettachment check is performed inside the NFT Manager contract