sherlock-audit / 2023-02-bond-judging

2 stars 0 forks source link

ss3434 - ERC-20's transfer function return value not checked. #9

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ss3434

medium

ERC-20's transfer function return value not checked.

Summary

ERC-20's transfer function return value not checked.

Vulnerability Detail

Some erc20 tokens doesn't revert on transfer and their return value should be checked.

Impact

No use of safetransfer()

Code Snippet

https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondBatchAuctionV1.sol#L287

 function emergencyWithdraw(ERC20 token_) external override onlyOwner {
        // Confirm that the token is a contract
        if (address(token_).code.length == 0 && address(token_) != address(0))
            revert BatchAuction_InvalidParams();

        // If token address is zero, withdraw ETH
        if (address(token_) == address(0)) {
            payable(owner()).transfer(address(this).balance);
        } else {
            token_.safeTransfer(owner(), token_.balanceOf(address(this)));
        }
    }

Tool used

Manual Review

Recommendation

use safetransfer()

-        payable(owner()).transfer(address(this).balance);;
+        payable(owner()).safeTransfer(address(this).balance);
Oighty commented 1 year ago

Disagree with this finding. The transfer call referenced is an ETH transfer, not an ERC20 token transfer.