If multiple reward tokens configured, its possible to block withdrawTaxes of all rewardTokens

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/PermissionlessBasicPoolFactory.sol#L265-L271

Vulnerability details

In PermissionlessBasicPoolFactory.sol , in function withdrawTaxes , if there is any known/unknown issues with any one of the rewards token or malicious reward token, none of the other valid rewardTokens can be withdrawn by the globalBenefeciary.

The contract should allow the transfer of other valid rewardTokens to the globalBeneficiary

This issue is also valid for withdraw of rewards by users.

Impact

The globalBeneficiary cannot withdraw any amount of other valid rewardTokens.

Proof of Concept

Contract : PermissionlessBasicPoolFactory.sol Function : withdrawTaxes()

Recommended Mitigation Steps

Allow the transfer of other valid rewardTokens to the globalBeneficiary

Current code block in withdrawTaxes is as below

        bool success = true;
        for (uint i = 0; i < pool.rewardTokens.length; i++) {
            uint tax = taxes[poolId][i];
            taxes[poolId][i] = 0;
            success = success && IERC20(pool.rewardTokens[i]).transfer(globalBeneficiary, tax);
        }
        require(success, 'Token transfer failed');

The above code block to be changed to below code block

        for (uint i = 0; i < pool.rewardTokens.length; i++) {
            bool success = true;
            uint tax = taxes[poolId][i];
            taxes[poolId][i] = 0;
            if (tax > 0) {
               success = IERC20(pool.rewardTokens[i]).transfer(globalBeneficiary, tax);
               if (!success) {
                  taxes[poolId][i] = tax;
               }
            }
        }

[illuzen]

Duplicate #20