ydlee - A token owner cannot remove one mgCvg delegation when he already delegates to maxMgDelegatees addresses.

[sherlock-admin2]

ydlee

medium

A token owner cannot remove one mgCvg delegation when he already delegates to maxMgDelegatees addresses.

Summary

A token owner cannot remove one mgCvg delegation when he already delegates to maxMgDelegatees addresses, which violates the functionality of delegateMgCvg function.

Vulnerability Detail

A token owner can delegate his mgCvg to up to maxMgDelegatees addresses through delegateMgCvg function. This function can also be used to remove a delegation by setting parameter _percentage to 0 (L273). The removing works fine when the number of delegatees is less than maxMgDelegatees. If the token owner already delegates his mgCvg to maxMgDelegatees addresses, he cannot remove one specific delegation with delegateMgCvg, as the number of existing delegatees is required less than maxMgDelegatees at the begining of delegateMgCvg function (L282). This is inconsistent with the functionality of delegateMgCvg function.
https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L271-L282

271:    /**
272:     * @notice Delegates a percentage of the mgCvG for a tokenId to another address (the mgCvg can be delegated to several addresses).
273:=>   * @dev Percentage=0 can be used to remove a delegation.
274:     * @param _tokenId is the ID of the token (NFT) to delegate voting power.
275:     * @param _to is the address we want to delegate to.
276:     * @param _percentage is the percentage we want to delegate to the address.
277:     */
278:    function delegateMgCvg(uint256 _tokenId, address _to, uint96 _percentage) external onlyTokenOwner(_tokenId) {
279:        require(_percentage <= 100, "INVALID_PERCENTAGE");
280:
281:        uint256 _delegateesLength = delegatedMgCvg[_tokenId].length;
282:=>      require(_delegateesLength < maxMgDelegatees, "TOO_MUCH_DELEGATEES");

POC: Let's add a new test case to sherlock-cvg/test/ut/delegation/manage-delegation.spec.ts to show this.

// File: sherlock-cvg/test/ut/delegation/manage-delegation.spec.ts

    it ("Delegates mgCvg to maxMgDelegatees and remove a delegation", async () => {
        // delegate mgCvg to 5 users to reach `maxMgDelegatees`
        await lockingPositionDelegate.connect(user1).delegateMgCvg(1, user4, 10);
        await lockingPositionDelegate.connect(user1).delegateMgCvg(1, user5, 10);
        await lockingPositionDelegate.connect(user1).delegateMgCvg(1, user6, 10);
        await lockingPositionDelegate.connect(user1).delegateMgCvg(1, user7, 10);
        await lockingPositionDelegate.connect(user1).delegateMgCvg(1, user8, 10);

        // remove one delegation through `delegateMgCvg` with 0 percentage -> revert
        await lockingPositionDelegate.connect(user1).delegateMgCvg(1, user6, 0).should.be.revertedWith("TOO_MUCH_DELEGATEES");

        // only able to remove one delegation by clean all delegations
        await lockingPositionDelegate.connect(user1).cleanDelegatees(1, false, true);
    });

In such case, token owner has to remove all the delegations first, and then add other delegations again except the one he wants to remove.

Impact

A token owner cannot remove one mgCvg delegation when he already delegates to maxMgDelegatees addresses, which violates the functionality of delegateMgCvg function.

Code Snippet

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L271-L282

Tool used

Manual Review

Recommendation

Change the maxMgDelegatees checking as follows:

@@ -279,7 +279,7 @@ contract LockingPositionDelegate is Initializable {
         require(_percentage <= 100, "INVALID_PERCENTAGE");
 
         uint256 _delegateesLength = delegatedMgCvg[_tokenId].length;
-        require(_delegateesLength < maxMgDelegatees, "TOO_MUCH_DELEGATEES");
+        require((_percentage > 0 && _delegateesLength < maxMgDelegatees) || (_percentage == 0 && _delegateesLength <= maxMgDelegatees), "TOO_MUCH_DELEGATEES");

Duplicate of #142