WrappedVotingNftMintStrategy::_distribute()

WrappedVotingNftMintStrategy::_distribute() - L209-211: DoS of token distribute (to winner) functionality due to deletion of the value of state variable poolAmount BEFORE transfer of pool tokens.

Vulnerability Detail

The _distribute() function will either revert due to _transferAmount() sending poolAmount of zero value, or it will succeed but zero tokens will be sent. Either way, winner will not be able to receive the pool tokens at all, via this function.

    /// @notice Internal function to distribute the tokens to the winner
    /// @param _sender The sender of the transaction
    function _distribute(address[] memory, bytes memory, address _sender) internal override onlyAfterAllocation {
        IAllo.Pool memory pool = allo.getPool(poolId);

        if (poolAmount == 0) {
            revert INVALID();
        }

        delete poolAmount;  /// @audit-issue this becomes zero/default value, i.e. `poolAmount` == 0 == true

        _transferAmount(pool.token, currentWinner, poolAmount);

        emit Distributed(currentWinner, currentWinner, poolAmount, _sender);
    }

Impact

Code Snippet

https://github.com/allo-protocol/allo-v2/blob/851571c27df5c16f6586ece2a1cb6fd0acf04ec9/contracts/strategies/_poc/wrapped-voting-nftmint/WrappedVotingNftMintStrategy.sol#L200-L214

Tool used

VSC. Manual Review

Recommendation

Recommendations:

    /// @notice Internal function to distribute the tokens to the winner
    /// @param _sender The sender of the transaction
    function _distribute(address[] memory, bytes memory, address _sender) internal override onlyAfterAllocation {
        IAllo.Pool memory pool = allo.getPool(poolId);

        if (poolAmount == 0) {
            revert INVALID();
        }

    ++  uint256 _poolAmount = poolAmount;
        delete poolAmount;

    --  _transferAmount(pool.token, currentWinner, poolAmount);
    ++  _transferAmount(pool.token, currentWinner, _poolAmount);

        emit Distributed(currentWinner, currentWinner, poolAmount, _sender);
    }

sherlock-audit / 2023-09-Gitcoin-judging