chaduke - QVSimpleStrategy.removeAllocator() fails to remove the votes casted by the allocator, leading to unfair fund distribution.

[sherlock-admin]

chaduke

high

QVSimpleStrategy.removeAllocator() fails to remove the votes casted by the allocator, leading to unfair fund distribution.

QVSimpleStrategy.removeAllocator() fails to remove the votes casted by the allocator. As a result, the vote information will be wrong after the allocator is removed and fund distribution will be unfair - some recipients will receive less funds than they are supposed to.

Vulnerability Detail

QVSimpleStrategy.removeAllocator() will remove an allocator:

https://github.com/allo-protocol/allo-v2/blob/0b881ef4a0013d2809374c9ea69f4cf1288dfe62/contracts/strategies/qv-simple/QVSimpleStrategy.sol#L97-L101

However, the votes by the allocator has already been casted by function _allocate():

https://github.com/allo-protocol/allo-v2/blob/0b881ef4a0013d2809374c9ea69f4cf1288dfe62/contracts/strategies/qv-simple/QVSimpleStrategy.sol#L107-L124

QVSimpleStrategy.removeAllocator() fails to modify:

1) totalRecipientVotes;

2) the votes for the recipients that the removed allocator has voted: _recipient.totalVotesReceived.

As a result, the payout for each recipient, poolAmount * recipient.totalVotesReceived / totalRecipientVotes; will not be calculated correctly, leading to some unfair fund distribution.

Impact

Code Snippet

``QVSimpleStrategy.removeAllocator() fails to remove the votes casted by the removed allocator, leading to unfair fund distribution.

Tool used

VScode

Manual Review

Recommendation

Both the totalRecipientVotes and _recipient.totalVotesReceived should be decreased accordingly when an allocator is removed.

[sherlock-admin]

1 comment(s) were left on this issue during the judging contest.

n33k commented:

invalid, low likelihood of removing allocator and low impact, dup of 902

[neeksec]

Duplicate of #902