FactoryDAO contest - 0x52's results

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/SpeedBumpPriceGate.sol#L79

Vulnerability details

Impact

Contract overcharges user

Proof of Concept

L79 passes through the entire ether balance sent. Since price decays each block it is likely that the transaction won't be processed during the same block that it was submitted. In situations like this the price at transaction confirmation will be lower (due to decay) than the amount of ether sent

Tools Used

Recommended Mitigation Steps

passThruGate should only take calculated price and should either return excess or allow excess to be claimed using a separate refund function

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/PermissionlessBasicPoolFactory.sol#L144 https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/PermissionlessBasicPoolFactory.sol#L249-L252 https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/PermissionlessBasicPoolFactory.sol#L224-L230

Vulnerability details

Pool creator is able to specify any asset token so it is possible to create a pool with reward tokens that support fee on transfer

Impact

Portion of funds permanently stuck in contract

Proof of Concept

When pool is funded, total reward distribution is set to the amount transferred. When token supports fee on transfer, the total tokens received will be less the fee amount. This means the contract has less of the token than it expects. This leads to one of two outcomes. If the reward schedule finishes and there are excess tokens then the following code will fail when trying to transfer the fee on transfer token:

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

This would cause all excess rewards of all token types to be permanently stuck with no way to recover them. In the second case where all rewards are allocated, the last receipt to withdraw will always fail at the following lines because there will not be enough of the fee to transfer token to complete the transfer because of the fee taken during transfer to the pool:

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

This would cause all funds associated with receipt to be permanently locked in the contract with no way to recover them

Tools Used

Recommended Mitigation Steps

During pool funding, set reward funding to the actual amount of tokens received from the transfer rather than the specified transfer amount

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/PermissionlessBasicPoolFactory.sol#L242-L256 https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/PermissionlessBasicPoolFactory.sol#L189-L196

Vulnerability details

Impact

Attacker can DOS withdrawExcessRewards keeping excess funds locked in contract until huge gas fees are paid

Proof of Concept

All receipts must be cashed out before pool creator can call withdrawExcessRewards() because of the check below:

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

A new receipt is created for each deposit in the lines below:

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

This would allow the attacker to create lots of tiny deposits (i.e. 1 wei each). This would cost a large amount of gas to remove since each withdrawal would require a minimum of 2 token transfers (1 for underlying and 1 minimum rewards). This cost increasing higher and higher with each additional reward token.

Tools Used

Recommended Mitigation Steps

Allow pool creator to set minimum deposit size or update deposit function to payout accumulated rewards when an address repeat deposits