the ERC20 .transfer() method is used in MerkleVesting.sol, but its return value is not checked.
It is good to add a require() statement that checks the return value of token transfers, or to use something like OpenZeppelin’s safeTransfer. Failure to do so will cause silent failures of transfers, which will be confusing for users calling these functions, and may result in loss of funds.
If the transfer fails, the function will not revert. As the tree balance is updated line 167, this would result in a user not being able to retrieve their tokens.
illuzen
commented
2 years ago
Lines of code
https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleVesting.sol#L173
Vulnerability details
ERC20: TRANSFER() RETURN VALUE NOT CHECKED
the ERC20
.transfer()method is used inMerkleVesting.sol, but its return value is not checked. It is good to add a require() statement that checks the return value of token transfers, or to use something like OpenZeppelin’s safeTransfer. Failure to do so will cause silent failures of transfers, which will be confusing for users calling these functions, and may result in loss of funds.Impact
Medium
Proof Of Concept
Instances include:
If the transfer fails, the function will not revert. As the tree balance is updated line 167, this would result in a user not being able to retrieve their tokens.
Tools Used
Manual Analysis
Recommended Mitigation Steps
use
safeTransferorrequire()consistently.