Upon pool creation, the pool configures a taxPerCapita value which is controlled by the global beneficiary. This global beneficiary account can effectively sandwich calls to addPool() by increasing the fee to 100% before the addition of a new pool and subsequently decreasing the fee after pool creation. As a result, all reward tokens allocated by the pool creator can be stolen by the global beneficiary. Alternatively, this can be used as a way to censor certain accounts from creating pools.
Recommended Mitigation Steps
Consider putting the setGlobalTax() function behind a timelock. This will ensure pool creators are unable to be sandwiched by a malicious global beneficiary. It makes sense for this to be on-chain as it allows users to be aware of any changes to sensitive parameters.
Lines of code
https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L314-L318 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L110 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L227
Vulnerability details
Impact
Upon pool creation, the pool configures a
taxPerCapita
value which is controlled by the global beneficiary. This global beneficiary account can effectively sandwich calls toaddPool()
by increasing the fee to 100% before the addition of a new pool and subsequently decreasing the fee after pool creation. As a result, all reward tokens allocated by the pool creator can be stolen by the global beneficiary. Alternatively, this can be used as a way to censor certain accounts from creating pools.Recommended Mitigation Steps
Consider putting the
setGlobalTax()
function behind a timelock. This will ensure pool creators are unable to be sandwiched by a malicious global beneficiary. It makes sense for this to be on-chain as it allows users to be aware of any changes to sensitive parameters.