code-423n4 / 2023-08-verwa-findings

8 stars 7 forks source link

Voters can vote on a single pool multiple times by redelegating #459

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L356-L387 https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/GaugeController.sol#L211-L278

Vulnerability details

Impact

Users should be able to have only one concurrent vote on a pool in GaugeController. When a user votes the weight of his vote is calculated using his _user_weight parameter and the slope and end time of his balance lock are used to calculate the bias for that pool.

But nothing stops him from delegating his balance to another voter who can then use it to vote in GaugeController. This means that many delegators can vote individually on a pool and also delegate their balance to another voter who uses all their balances to vote on the same pool. The time of delegation does not matter i.e before or after individual votes. This effectively allows them to have multiple votes on a pool. It can also be carried out by an individual user through multiple iterations of the following proof of concept.

Proof of Concept

Alice creates an attacker contract to:

  1. Vote on a pool.
  2. Delegate to another address of hers. The address can lock as low as 1*e-18 CANTO (assuming CANTO has 18 decimal places) to become a valid voter.
  3. Vote using that address. This address can be a contract created cheaply using Openzeppelin's MinimalProxy, an implementation of EIP 1167.
  4. Repeat from no. 2.

Tools Used

VsCode

Recommended Mitigation Steps

Depending on what the team actually wants, two different steps can be taken.

  1. Set the slope to zero whenever a user delegates in VotingEscrow. This will ensure the vote weight of a wallet that delegates is zero.
  2. Modify GaugeController to check the delegatee of a msg.sender by calling the locked function on VotingEscrow. GaugeController can revert if delegatee != msg.sender

Assessed type

Other

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #45

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #99

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #178

c4-pre-sort commented 1 year ago

141345 marked the issue as not a duplicate

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #86

c4-judge commented 1 year ago

alcueca changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

alcueca changed the severity to 3 (High Risk)

c4-judge commented 1 year ago

alcueca marked the issue as partial-50