carrotsmuggler - QVSimpleStrategy.sol: `allocator.voiceCredits` is never updated

[sherlock-admin2]

carrotsmuggler

high

QVSimpleStrategy.sol: allocator.voiceCredits is never updated

QVSimpleStrategy.sol: allocator.voiceCredits is never updated. This is used in the function _hasVoiceCreditsLeft to check if they have votes left, but this value is never updated, so the limit is never hit.

Vulnerability Detail

The function _allocate in QVSimpleStrategy.sol is used to allcoate votes for a recipient. This is done using quadratic voting. In the _allocate function, first the caller is checked to make sure they are eligibile to vote. Then the function _hasVoiceCreditsLeft is called to check if the voter has any voting credits left. If so, they can assign their votes to a specific recipient.

function _allocate(bytes memory _data, address _sender) internal virtual override {

    // check that the recipient has voice credits left to allocate
    if (!_hasVoiceCreditsLeft(voiceCreditsToAllocate, allocator.voiceCredits)) revert INVALID();
    _qv_allocate(allocator, recipient, recipientId, voiceCreditsToAllocate, _sender);
}

Any allocator can allocate some of their votes to any eligible recipient. The function _hasVoiceCreditsLeft makes sure that a single allocator does not put in more votes than the max limit.

function _hasVoiceCreditsLeft(uint256 _voiceCreditsToAllocate, uint256 _allocatedVoiceCredits)
        internal
        view
        override
        returns (bool)
    {
        return _voiceCreditsToAllocate + _allocatedVoiceCredits <= maxVoiceCreditsPerAllocator;
    }

For this check, the allocator.voiceCredits is used. However, the issue is that after a vote, this allocator.voiceCredits is not updated to reflect the votes put in. The allocator struct is updated in the _qv_allocate function. The update steps are shown below.

voteResult -= votesCastToRecipient;
totalRecipientVotes += voteResult;
_recipient.totalVotesReceived += voteResult;

_allocator.voiceCreditsCastToRecipient[_recipientId] += totalCredits;
_allocator.votesCastToRecipient[_recipientId] += voteResult;

Only the voiceCreditsCastToRecipient and votesCastToRecipient are updated. The allocator.voiceCredits is not updated. This means the _hasVoiceCreditsLeft function will always return true, since allocateor.voiceCredits is always 0.

Impact

The maxVoiceCreditsPerAllocator check is never done. Allocators can vote multiple times for recipients without any upper limit.

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/strategies/qv-base/QVBaseStrategy.sol#L506-L534

Tool used

Manual Review

Recommendation

Update allocator.voiceCredits when the other fields are being updated.

allocator.voiceCredits += totalCredits;

Duplicate of #150