hash - User's can attain unlimited `veCvg/mgCvg` voting power due to lack of duplication checks

[sherlock-admin]

hash

high

User's can attain unlimited veCvg/mgCvg voting power due to lack of duplication checks

Summary

User's can attain unlimited veCvg/mgCvg voting power due to lack of duplication checks

Vulnerability Detail

The manageOwnedAndDelegated function doesn't check for duplication among the added tokens. This allows an user to add for oneself the same token infinite number of times. The mgCvgVotingPowerPerAddress and veCvgVotingPowerPerAddress functions which are used to compute the voting power from the tokenOwnedAndDelegated which was previously set by manageOwnedAndDelegated also doesn't check for duplication. This allows a user to gain infinite governance power.

Impact

Attackers can gain unlimited voting power on governance disrupting the entire protocol

Code Snippet

The manageOwnedAndDelegated function doesn't check for duplication https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L330-L367

The mgCvgVotingPowerPerAddress function doesn't check for duplication https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionService.sol#L773-L821

The veCvgVotingPowerPerAddress function doesn't check for duplication https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionService.sol#L727-L767

Tool used

Manual Review

Recommendation

Check for duplication in the all arrays. Since this is being set externally and the newly minted tokens having the latest tokenId, enforcing an increasing tokenId in arrays can solve this issue.

+++     uint prevTokenId;
        for (uint256 i; i < _ownedAndDelegatedTokens.owneds.length;) {
+++         require(_ownedAndDelegatedTokens.owneds[i] > prevTokenId);
            /** @dev Check if tokenId is owned by the user.*/
            require(
                msg.sender == cvgControlTower.lockingPositionManager().ownerOf(_ownedAndDelegatedTokens.owneds[i]),
                "TOKEN_NOT_OWNED"
            );
            tokenOwnedAndDelegated[msg.sender].owneds.push(_ownedAndDelegatedTokens.owneds[i]);
            unchecked {
                ++i;
            }
        }

Similarly for the other 2 arrays

Duplicate of #126