Omniscia Keyko Audit
MerkleDistributor Static Analysis Findings
MerkleDistributor Static Analysis Findings
MDR-01S: Improper Safe Invocation of ERC-20 Transfer
| Type | Severity | Location |
|---|---|---|
| Standard Conformity | Minor | MerkleDistributor.sol:L68, L75-L78 |
Description:
The EIP-20 standard dictates that callers of the transfer function should not assume that a bool is not returned, however, certain tokens are actually non-complient (i.e. USDT / Tether) and would cause the code segment to halt execution given that no bool is returned by that token.
Example:
contracts/airgrab/MerkleDistributor.sol
51function claim(52 uint256 index,53 address account,54 uint256 amount,55 bytes32[] calldata merkleProof56) external override withinClaimPeriod {57 require(!isClaimed(index), "MerkleDistributor: Drop already claimed.");58
59 // Verify the merkle proof.60 bytes32 node = keccak256(abi.encodePacked(index, account, amount));61 require(62 MerkleProof.verify(merkleProof, merkleRoot, node),63 "MerkleDistributor: Invalid proof."64 );65
66 // Mark it claimed and send the token.67 _setClaimed(index);68 require(IERC20(token).transfer(account, amount), "MerkleDistributor: Transfer failed.");69
70 emit Claimed(index, account, amount);71}72
73function withdrawUnclaimed() external override onlyOwner claimPeriodEnded {74 uint256 unclaimedBalance = IERC20(token).balanceOf(address(this));75 require(76 IERC20(token).transfer(msg.sender, unclaimedBalance),77 "MerkleDistributor: Withdrawal failed."78 );79 emit Withdrawn(msg.sender, unclaimedBalance);80}Recommendation:
As there are other token implementations that also deviate from the EIP-20 standard, we strongly recommend the usage of a safe wrapper library such as SafeERC20 from OpenZeppelin which opportunistically evaluates the returned bool variable only if it exists.
Alleviation:
The safeTransfer implementation is now properly utilised in all instances instead.