sherlock-audit / 2023-11-convergence-judging

8 stars 8 forks source link

lemonmon - `LockingPositionService`'s voting power can be inflated by putting duplicated entries to `LockingPositionDelegate.tokenOwnedAndDelegated` #196

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

lemonmon

medium

LockingPositionService's voting power can be inflated by putting duplicated entries to LockingPositionDelegate.tokenOwnedAndDelegated

Summary

A malicious user can put duplicated entries to LockingPositionDelegate.tokenOwnedAndDelegated and get more voting power (LockingPositionService.veCvgVotingPowerPerAddress() or LockingPositionService.mgCvgVotingPowerPerAddress()) than what they should get, therefore potentially rig the Cvg-governance or Meta-governance.

Vulnerability Detail

LockingPositionService.veCvgVotingPowerPerAddress uses the returned arrays from LockingPostionDelegate.getTokenVeOwnedAndDelegated:

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionService.sol#L732-L733

In the LockingPositionService.veCvgVotingPowerPerAddress, the returned arrays will be checked in loops and the voting power will be counted for the user.

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionService.sol#L736-L765

However, LockingPositionService.veCvgVotingPowerPerAddress does not check whether certain tokenId is already counted for the user.

The arrays above are coming from LockingPostionDelegate.tokenOwnedAndDelegated, which can be updated via LockingPositionDelegate.manageOwnedAndDelegated by users. The function as well does not check whether the given arrays contain duplicated entries.

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

The same problem occurs with LockingPositionService.mgCvgVotingPowerPerAddress as well. Here the function fails to check for duplicated entries and will simply add voting powers even if there are any duplicated entries

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionService.sol#L773

Impact

According to the comments, these functions LockingPositionService.mgCvgVotingPowerPerAddress and LockingPositionService.veCvgVotingPowerPerAddress are used in Meta-governance strategy and Cvg Governance proposal strategy.

A malicious user can manipulate these governances by inflating their voting power.

Code Snippet

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionService.sol#L732-L733 https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionService.sol#L736-L765

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

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Locking/LockingPositionService.sol#L773

Tool used

Manual Review

Recommendation

in the LockingPositionDelegate.manageOwnedAndDelegated function, check and do not allow to have duplicates.

Duplicate of #126