sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

simon135 - since `_allocator.voiceCreidtsCastToReceipeints+=totalCredits` is wrong and will be infliated not like the spec #854

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 12 months ago

simon135

high

since _allocator.voiceCreidtsCastToReceipeints+=totalCredits is wrong and will be infliated not like the spec

since there is a wrong adding on to voiceCreditsCastToRecipeints the recipients can get substantially more votes

Vulnerability Detail

so for example If Alice the allocator gives 5 credits to Bob then a day later Alice gives another 5 credits to Bob bobs total credits=15 which it should not be. so bob total votes=3162277660 = sqrt(10e18) Then sam the allocator gives 1 credit to Bob which should only be 11 credits but instead he gets credits 16 which is 4000000000 votes

This is because

        uint256 creditsCastToRecipient = _allocator.voiceCreditsCastToRecipient[_recipientId];
        // get the total credits and calculate the vote result
        uint256 totalCredits = _voiceCreditsToAllocate + creditsCastToRecipient;
        uint256 voteResult = _sqrt(totalCredits * 1e18);
_allocator.voiceCreditsCastToRecipient[_recipientId] += totalCredits;

one the first call from Alice it will be correct and voiceCreditsCastToRecipient=5 The second call creditsCastToReceipient=5 totalCredits=5+5 so when we update voiceCreditsCastToReceipint+=10 it going to =15

Impact

  1. The allocator can do this on purpose to get their favorite recipient more credits exponentially and then someone else allocates 1 credit giving the recipient way more votes or the allocator can just use 9 credits and then use 1 getting more votes then when _distribute is called the recipient will get more funds since they got more votes

    Code Snippet

    
    // check the `_voiceCreditsToAllocate` is > 0
        if (_voiceCreditsToAllocate == 0) revert INVALID();
    
        // get the previous values
        uint256 creditsCastToRecipient = _allocator.voiceCreditsCastToRecipient[_recipientId];
        uint256 votesCastToRecipient = _allocator.votesCastToRecipient[_recipientId];
    
        // get the total credits and calculate the vote result
        uint256 totalCredits = _voiceCreditsToAllocate + creditsCastToRecipient;
        uint256 voteResult = _sqrt(totalCredits * 1e18);
    
        // update the values
        voteResult -= votesCastToRecipient;
        totalRecipientVotes += voteResult;
        _recipient.totalVotesReceived += voteResult;
    // @audit we are += the totalCredits instead of = which causes the issue 
        _allocator.voiceCreditsCastToRecipient[_recipientId] += totalCredits;
        _allocator.votesCastToRecipient[_recipientId] += voteResult;
    
        // emit the event with the vote results
        emit Allocated(_recipientId, voteResult, _sender);
    }
https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/strategies/qv-base/QVBaseStrategy.sol#L529
## Tool used

Manual Review

## Recommendation
instead of +=
it should be = 

```solidity
_allocator.voiceCreditsCastToRecipient[_recipientId]= totalCredits;

Duplicate of #48