search

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

1 stars 1 forks source link

Missing ERC20 return value check in `MerkleVesting#withdraw` #274

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/e22a562c01c533b8765229387894cc0cb9bed116/contracts/MerkleVesting.sol#L169-L174

Vulnerability details

MerkleVesting#withdraw does not check the return value of the token withdrawal on line 173. If an ERC20 token returns false to indicate a failed transfer but does not revert, this transfer will silently fail but the withdrawal amount will still be deducted from the user's stored tokenBalance.

MerkleVesting#withdraw

        // Transfer the tokens, if the token contract is malicious, this will make the whole tree malicious
        // but this does not allow re-entrance due to struct updates and it does not effect other trees.
        // It is also consistent with the ethereum general security model:
        // other contracts do what they want, it's our job to protect our contract
        IERC20(tree.tokenAddress).transfer(destination, currentWithdrawal);
        emit WithdrawalOccurred(treeIndex, destination, currentWithdrawal, tranche.currentCoins);

Suggestion: use OpenZeppelin's SafeERC20 helper library to ensure nonstandard token transfers revert with an error.

illuzen commented 2 years ago

Duplicate #27