tsvetanovv - Use `call()` instead of `transfer()` when transferring ETH

[sherlock-admin]

tsvetanovv

medium

Use call() instead of transfer() when transferring ETH

Summary

call() should be used instead of transfer() on an address payable.

Vulnerability Detail

The transfer() and send() functions forward a fixed amount of 2300 gas. Historically, it has often been recommended to use these functions for value transfers to guard against reentrancy attacks. However, the gas cost of EVM instructions may change significantly during hard forks which may break already deployed contract systems that make fixed assumptions about gas costs.

Impact

See Vulnerability Detail

Code Snippet

https://github.com/sherlock-audit/2023-03-sense/blob/main/sense-v1/pkg/core/src/Periphery.sol#L1008-L1014

function _transfer(
        ERC20 token,
        address receiver,
        uint256 amt
    ) internal {
        address(token) == ETH ? payable(receiver).transfer(amt) : token.safeTransfer(receiver, amt); //@audit - use call
    }

https://github.com/sherlock-audit/2023-03-sense/blob/main/auto-roller/src/RollerPeriphery.sol#L111

110:    address(quote.buyToken) == ETH
111:            ? payable(receiver).transfer(amtOut) //@audit - use call
112:            : ERC20(address(quote.buyToken)).safeTransfer(receiver, amtOut);

260: payable(msg.sender).transfer(refundAmt);

Tool used

Manual Review

Recommendation

I recommend using call() method instead of transfer().

Duplicate of #33