sherlock-audit / 2022-11-frankendao-judging

1 stars 0 forks source link

0x52 - Adversary can abuse delegating to lower quorum #24

Open github-actions[bot] opened 1 year ago

github-actions[bot] commented 1 year ago

0x52

high

Adversary can abuse delegating to lower quorum

Summary

When a user delegates to another user they surrender their community voting power. The quorum threshold for a vote is determined when it is created. Users can artificially lower quorum by delegating to other users then creating a proposal. After it's created they can self delegate and regain all their community voting power to reach quorum easier.

Vulnerability Detail

// If a user is delegating back to themselves, they regain their community voting power, so adjust totals up
if (_delegator == _delegatee) {
  _updateTotalCommunityVotingPower(_delegator, true);

// If a user delegates away their votes, they forfeit their community voting power, so adjust totals down
} else if (currentDelegate == _delegator) {
  _updateTotalCommunityVotingPower(_delegator, false);
}

When a user delegates to user other than themselves, they forfeit their community votes and lowers the total number of votes. When they self delegate again they will recover all their community voting power.

    newProposal.id = newProposalId.toUint96();
    newProposal.proposer = msg.sender;
    newProposal.targets = _targets;
    newProposal.values = _values;
    newProposal.signatures = _signatures;
    newProposal.calldatas = _calldatas;

    //@audit quorum votes locked at creation

    newProposal.quorumVotes = quorumVotes().toUint24();
    newProposal.startTime = (block.timestamp + votingDelay).toUint32();
    newProposal.endTime = (block.timestamp + votingDelay + votingPeriod).toUint32();

When a proposal is created the quorum is locked at the time at which it's created. Users can combine these two quirks to abuse the voting.

Example:

Assume there is 1000 total votes and quorum is 20%. Assume 5 users each have 35 votes, 10 base votes and 25 community votes. In this scenario quorum is 200 votes which they can't achieve. Each user delegates to other users, reducing each of their votes by 25 and reducing the total number of votes of 875. Now they can create a proposal and quorum will now be 175 votes (875*20%). They all self delegate and recover their community votes. Now they can reach quorum and pass their proposal.

Impact

Users can collude to lower quorum and pass proposal easier

Code Snippet

https://github.com/sherlock-audit/2022-11-frankendao/blob/main/src/Staking.sol#L286-L316

Tool used

Manual Review

Recommendation

One solution would be to add a vote cooldown to users after they delegate, long enough to make sure all active proposals have expired before they're able to vote. The other option would be to implement checkpoints.

zobront commented 1 year ago

This is clever but the impact that a small number of users can have on quorum is relatively small, and we aren't concerned.

IAm0x52 commented 1 year ago

Escalate for 1 USDC

This is a valid issue. Even if they don't want to fix, it still has an impact on quorum which impacts proposals passing or failing. The impact is small at low quorum thresholds but at higher quorum thresholds the impact is larger.

As an example if the quorum threshold is 10% then using this technique to drop overall votes by 100 would lower quorum by 10 votes but if the threshold is 50% then it would lower quorum by 50 votes.

This decrease in quorum could allow borderline proposals to pass that normally couldn't meet quorum. Since the impact depends on the current quorum threshold (which is adjustable), medium seems appropriate to me.

sherlock-admin commented 1 year ago

Escalate for 1 USDC

This is a valid issue. Even if they don't want to fix, it still has an impact on quorum which impacts proposals passing or failing. The impact is small at low quorum thresholds but at higher quorum thresholds the impact is larger.

As an example if the quorum threshold is 10% then using this technique to drop overall votes by 100 would lower quorum by 10 votes but if the threshold is 50% then it would lower quorum by 50 votes.

This decrease in quorum could allow borderline proposals to pass that normally couldn't meet quorum. Since the impact depends on the current quorum threshold (which is adjustable), medium seems appropriate to me.

You've created a valid escalation for 1 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

zobront commented 1 year ago

I start by that this wouldn't have any substantial impact. First off, if a user delegates, they won't have any votes and therefore won't meet the threshold to create a proposal. So it would need to be a coordinated effort. And in the event that this happened, the impact would be relatively small. Seems unlikely and not impactful enough that a Medium feels like a stretch.

Evert0x commented 1 year ago

Escalation accepted. Assigning medium severity as the impact is not that significant + it requires a lot of effort to execute.

The quorum can be manipulated which can lead to unexpected behavior as illustrated.

sherlock-admin commented 1 year ago

Escalation accepted. Assigning medium severity as the impact is not that significant + it requires a lot of effort to execute.

The quorum can be manipulated which can lead to unexpected behavior as illustrated.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.