code-423n4 / 2022-05-factorydao-findings

1 stars 1 forks source link

PermissionlessBasicPoolFactory.sol excessBeneficiary can't withdraw until all deposits are withdrawn #196

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L245

Vulnerability details

Impact

Excess rewards can't be withdraw until all deposits are withdrawn. It is possible to withdraw rewards for other users after pool.endTime however that could require a lot of effort, gas and from security perspective "pull" mechanism is much better than "push". There will be always a significant amount of excess rewards because:

Recommended Mitigation Steps

Change the logic of withdrawExcessRewards() so it would be possible to withdraw excess reward proportionally to the pool.totalDepositsWei if all deposits are not withdrawn https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L242

illuzen commented 2 years ago

This is explicitly addressed in the comments of the contract. Keeping track of everyone's rewards is a linear problem, not feasible for smart contracts.

gititGoro commented 2 years ago

Marking invalid