sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

vagrant - QVBaseStrategy does not update poolAmount value after distributing tokens #892

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

vagrant

high

QVBaseStrategy does not update poolAmount value after distributing tokens

QVBaseStrategy does not update poolAmount value after distributing tokens

Vulnerability Detail

Depositing/funding the pool increases the poolAmount variable and the token balance of the contract but distributing tokens transfers the tokens to recipients and decreases the amount of tokens in the strategy/pool yet the poolAmount variable is not adjusted:

    function _distribute(address[] memory _recipientIds, bytes memory, address _sender)
        internal
        virtual
        override
        onlyPoolManager(_sender)
        onlyAfterAllocation
    {
        uint256 payoutLength = _recipientIds.length;
        for (uint256 i; i < payoutLength;) {
            address recipientId = _recipientIds[i];
            Recipient storage recipient = recipients[recipientId];

            PayoutSummary memory payout = _getPayout(recipientId, "");
            uint256 amount = payout.amount;

            if (paidOut[recipientId] || !_isAcceptedRecipient(recipientId) || amount == 0) {
                revert RECIPIENT_ERROR(recipientId);
            }

            IAllo.Pool memory pool = allo.getPool(poolId);
            _transferAmount(pool.token, recipient.recipientAddress, amount); 

            paidOut[recipientId] = true;

            emit Distributed(recipientId, recipient.recipientAddress, amount, _sender); // @audit poolAmount should be updated after distribute
            unchecked {
                ++i;
            }
        }
    }

Impact

QVBaseStrategy uses poolAmount to calculate how much it will distribute in _getPayout():

    function _getPayout(address _recipientId, bytes memory)
        internal
        view
        virtual
        override
        returns (PayoutSummary memory)
    {
        Recipient memory recipient = recipients[_recipientId];

        // Calculate the payout amount based on the percentage of total votes
        uint256 amount;
        if (!paidOut[_recipientId] && totalRecipientVotes != 0) {
            amount = poolAmount * recipient.totalVotesReceived / totalRecipientVotes;
        }
        return PayoutSummary(recipient.recipientAddress, amount);
    }

Not adjusting the poolAmount after distributing will lead to incorrect calculation of following distributions

Code Snippet

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

Tool used

Manual Review

Recommendation

adjust the poolAmount in QVBaseStrategy#_distribute()

sherlock-admin commented 1 year ago

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

n33k commented:

invalid, QVBaseStrategy are designed to distribute once