onDettachFromManagedNFT lockedRewards are taken from the managed locked balance

[hats-bug-reporter[bot]]

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

Description: Description\ When we attach our tokens to the managed our locked balance is added to the managed token in VotingEscrowUpgradeableV1_2::onAttachToManagedNFT and then additionally attach our token to one of the Strategies withManagedNFTStrategy::onAttach function:

function onAttachToManagedNFT(uint256 tokenId_, uint256 managedTokenId_) external onlyVoter {
      ManagedTokenInfo memory managedTokenInfo = managedTokensInfo[managedTokenId_];
      if (!managedTokenInfo.isManaged) {
          revert NotManagedNFT();
      }

      if (managedTokenInfo.isDisabled) {
          revert ManagedNFTIsDisabled();
      }

      if (managedTokensInfo[tokenId_].isManaged || tokensInfo[tokenId_].isAttached) {
          revert IncorrectUserNFT();
      }

      uint256 userBalance = IVotingEscrowV1_2(votingEscrow).onAttachToManagedNFT(tokenId_, managedTokenId_);
      tokensInfo[tokenId_] = TokenInfo(true, managedTokenId_, userBalance);

      IManagedNFTStrategy(IVotingEscrow(votingEscrow).ownerOf(managedTokenId_)).onAttach(tokenId_, userBalance);
  }

But when we detach the order is inversed, first ManagedNFTStrategy::onDetach is called which harvests the position and lockedRewards are added to the initial amount:

function onDettachFromManagedNFT(uint256 tokenId_) external onlyVoter {
      TokenInfo memory tokenInfo = tokensInfo[tokenId_];

      if (!tokenInfo.isAttached) {
          revert NotAttached();
      }

      assert(tokenInfo.attachedManagedTokenId != 0);

      uint256 lockedRewards = IManagedNFTStrategy(IVotingEscrow(votingEscrow).ownerOf(tokenInfo.attachedManagedTokenId)).onDettach(
          tokenId_,
          tokenInfo.amount
      );

      IVotingEscrowV1_2(votingEscrow).onDettachFromManagedNFT(
          tokenId_,
          tokenInfo.attachedManagedTokenId,
          tokenInfo.amount + lockedRewards
      );

      delete tokensInfo[tokenId_];
  }

and then onDettachFromManagedNFT is called which will decrease more than the added amount, leaving the managed nft with lower locked amount than initially.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

1. Proof of Concept (PoC) File
2. Revised Code File (Optional)

[0xmahdirostami]

POC, please

[BohdanHrytsak]

The amount that is added on top of the user's balance is their reward and does not leave mVeNFT with less than expected, other users' funds do not suffer.

At the exit, the user receives his balance that he has wagered + the accumulated reward, which is added to his mVeNFT, so it looks like everything is fine