Pool Creators Can Reject Taxes From Being Withdrawn

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L261-L272 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L224-L231

Vulnerability details

Impact

Upon pool creation, stakers can opt to earn rewards by staking their deposit tokens. When these deposit tokens are withdrawn, reward tokens are sent to the owner of the receipt along with their deposit tokens. A tax is charged on any rewards accrued by the user and are allocated to the global beneficiary. By calling the withdrawTaxes() function, taxes can be withdrawn by iterating through a list of reward tokens. A single transfer failure will cause the entire function call to revert, preventing the global beneficiary from receiving their tokens on a given pool. As a result, pool creators can use this knowledge by adding a malicious token to their list of reward tokens. This token can do nothing except revert when the recipient is the global beneficiary. As a result, this malicious token will protect stakers and ensure that they receive rewards but prevent the global beneficiary from receiving their allotted tokens.

Recommended Mitigation Steps

Consider utilising try and catch statements in withdrawTaxes() to ensure failed token transfers do not revert and prevent the beneficiary with withdrawing taxes earned on other reward tokens. Transfer failures will instead fail silently, allowing the beneficiary to withdraw their allocated tokens.

[illuzen]

Duplicate #20

[gititGoro]

reducing severity as rewards are not base assets.

[0xleastwood]

Isn't this a dupe of #124?