sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

0xnirlin - Rft Committee Strategy is not reusable and become useless after one recipient have been voted upon #968

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 11 months ago

0xnirlin

high

Rft Committee Strategy is not reusable and become useless after one recipient have been voted upon

Rft Committee Strategy is not reusable and become useless after one recipient have been voted upon as after the allocation adn distribution acceptedRecipientId there is no way to reset it.

Vulnerability Detail

As the acceptedRecipientId cannot be reset after the allocation and distribution.

Lets say alice is first recipient, have been voted on and distribution is done,

Not we want to vote on ewn recipient and do the allocation for it, but it is not possible as acceptedRecipientId still holds the alice address and will revert on the following line:

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/strategies/rfp-committee/RFPCommitteeStrategy.sol#L103

So the contract becomes useless and cannot be ever used for more recipients.

function _allocate(bytes memory _data, address _sender) internal override onlyPoolManager(_sender) {
        // @audit - allocation can be only done once, for upcoming milestones and recipient id's it will be always set to some address for the previous cycle, KOOL
        if (acceptedRecipientId != address(0)) {
            revert RECIPIENT_ALREADY_ACCEPTED();
        }

        // Check if the allocator has already casted the vote
        address voteCastedTo = votedFor[_sender];
        if (voteCastedTo != address(0)) {
            // remove the old vote to allow recasting of vote
            votes[voteCastedTo] -= 1;
        }

        // Decode the '_data'
        address recipientId = abi.decode(_data, (address));

        // Increment the votes for the recipient
        votes[recipientId] += 1;
        // Update the votedFor mapping
        votedFor[_sender] = recipientId;

        // Emit the event
        emit Voted(recipientId, _sender);

        // Check if the recipient has reached the vote threshold
        if (votes[recipientId] == voteThreshold) {
            // Set the accepted recipient
            acceptedRecipientId = recipientId;

            Recipient storage recipient = _recipients[acceptedRecipientId];
            recipient.recipientStatus = Status.Accepted;

            // Set the pool to inactive
            _setPoolActive(false);

            emit Allocated(acceptedRecipientId, recipient.proposalBid, allo.getPool(poolId).token, address(0));
        }
    }

Impact

Contract become useless after one voting session and can never be used again leading to costly redeployment on strategy whenever want to do voting on new address.

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/strategies/rfp-committee/RFPCommitteeStrategy.sol#L103

Tool used

Manual Review

Recommendation

After destribution set the acceptedRecipientId back to zero and open the option to set it again to some new acceptedRecipientId

sherlock-admin commented 11 months ago

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

n33k commented:

invalid, design choice