Lines of code
<https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L252&gt;

Vulnerability details

Impact

In PermissionlessBasicPoolFactory\addPool(), it doesn’t check whether pool.excessBeneficiary is address(0). Therefore, when doing withdrawExcessRewards(). IERC20(pool.rewardTokens[i]).transfer(pool.excessBeneficiary, rewards) always revert.

Proof of Concept

excessBeneficiary can be address(0)

<https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L92-L132&gt;

    function addPool (
        uint startTime,
        uint maxDeposit,
        uint[] memory rewardsWeiPerSecondPerToken,
        uint programLengthDays,
        address depositTokenAddress,
        address excessBeneficiary,
        address[] memory rewardTokenAddresses,
        bytes32 ipfsHash,
        bytes32 name
    ) external {
        …
        pool.excessBeneficiary = excessBeneficiary;
        …
    }

IERC20(pool.rewardTokens[i]).transfer(pool.excessBeneficiary, rewards); will always revert if pool.excessBeneficiary is address(0)

<https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L252&gt;

    function withdrawExcessRewards(uint poolId) external {
        Pool storage pool = pools[poolId];
        require(pool.id == poolId, 'Uninitialized pool');
        require(pool.totalDepositsWei == 0, 'Cannot withdraw until all deposits are withdrawn');
        require(block.timestamp > pool.endTime, 'Contract must reach maturity');

        bool success = true;
        for (uint i = 0; i < pool.rewardTokens.length; i++) {
            uint rewards = pool.rewardFunding[i];
            pool.rewardFunding[i] = 0;
            success = success && IERC20(pool.rewardTokens[i]).transfer(pool.excessBeneficiary, rewards);
        }
        require(success, 'Token transfer failed');
        emit ExcessRewardsWithdrawn(poolId);
    }

Tools Used

vim

Recommended Mitigation Steps

Add a null check for excessBeneficiaryaddress

e.g.

    function addPool (
        uint startTime,
        uint maxDeposit,
        uint[] memory rewardsWeiPerSecondPerToken,
        uint programLengthDays,
        address depositTokenAddress,
        address excessBeneficiary,
        address[] memory rewardTokenAddresses,
        bytes32 ipfsHash,
        bytes32 name
    ) external {
        require(excessBeneficiary != address(0), "excessBeneficiary cannot be the zero address")
        Pool storage pool = pools[++numPools];
        pool.id = numPools;
        pool.rewardsWeiPerSecondPerToken = rewardsWeiPerSecondPerToken;
        pool.startTime = startTime > block.timestamp ? startTime : block.timestamp;
        pool.endTime = pool.startTime + (programLengthDays * 1 days);
        pool.depositToken = depositTokenAddress;
        pool.excessBeneficiary = excessBeneficiary;
        pool.taxPerCapita = globalTaxPerCapita;

        require(rewardsWeiPerSecondPerToken.length == rewardTokenAddresses.length, 'Rewards and reward token arrays must be same length');

        // fill out the arrays with zeros
        for (uint i = 0; i < rewardTokenAddresses.length; i++) {
            pool.rewardTokens.push(rewardTokenAddresses[i]);
            pool.rewardsWeiClaimed.push(0);
            pool.rewardFunding.push(0);
            taxes[numPools].push(0);
        }
        pool.maximumDepositWei = maxDeposit;

        // this must be after pool initialization above
        fundPool(pool.id);

        {
            Metadata storage metadata = metadatas[numPools];
            metadata.ipfsHash = ipfsHash;
            metadata.name = name;
        }
        emit PoolAdded(pool.id, name, depositTokenAddress);
    }

---

The text was updated successfully, but these errors were encountered:

All reactions