search

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

1 stars 1 forks source link

In `MerkleVesting.sol::withdraw` check return value of ERC20 transfer or use `safeTransfer` of OZ #245

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/MerkleVesting.sol#L173

Vulnerability details

Impact

MerkleVesting.sol#L173

tree.tokenBalance -= currentWithdrawal;

IERC20(tree.tokenAddress).transfer(destination, currentWithdrawal);

In case of failed transfer here it do not check return value of transfer. it updates the tree balance without transfering the tokens. and causes loss.

Proof of Concept

MerkleVesting.sol#L173

Recommended Mitigation Steps

Use OpenZeppelin’s SafeERC20 safeTransfer instead of transfer that handles the return value check as well as non-standard-compliant tokens.

illuzen commented 2 years ago

Duplicate #27

itsmetechjay commented 2 years ago

Re-closing as duplicate.