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

1 stars 1 forks source link

USDT or other ERC20 incompatible token can not be used as reward token #229

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#L144

Vulnerability details

Proof of Concept

A simple POC: https://gist.github.com/wuwe1/9eb5bf9e4b3f31c8db52f4fa7fac5b13

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

    function fundPool(uint poolId) internal {
        Pool storage pool = pools[poolId];
        bool success = true;
        uint amount;
        for (uint i = 0; i < pool.rewardFunding.length; i++) {
            amount = getMaximumRewards(poolId, i);
            // transfer the tokens from pool-creator to this contract
            success = success && IERC20(pool.rewardTokens[i]).transferFrom(msg.sender, address(this), amount);

transferFrom will revert when pool.rewardTokens do not return boolean value as expected

Recommended Mitigation Steps

Use safeTransferFrom

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol#L29

illuzen commented 2 years ago

Duplicate #27