LineLib.sendOutTokenOrETH() may not be compatible with contract receiver because of use of transfer()

[code423n4]

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L48

Vulnerability details

Impact

LineLib.sendOutTokenOrETH() may revert when its receiver is a contract, preventing it from receiving ETH, and thus from much of the functionality of the protocol.

Proof of Concept

LineLib.sendOutTokenOrETH() is as follows.

/**
* @notice - Send ETH or ERC20 token from this contract to an external contract
* @param token - address of token to send out. Denominations.ETH for raw ETH
* @param receiver - address to send tokens to
* @param amount - amount of tokens to send
*/
function sendOutTokenOrETH(
    address token,
    address receiver,
    uint256 amount
)
    external
    returns (bool)
{
    if(token == address(0)) { revert TransferFailed(); }
    
    // both branches revert if call failed
    if(token!= Denominations.ETH) { // ERC20
        IERC20(token).safeTransfer(receiver, amount);
    } else { // ETH
        payable(receiver).transfer(amount);
    }
    return true;
}

In the case ETH is to be sent it calls payable(receiver).transfer(amount). transfer() forwards only 2300 gas which may cause a receiver contract to revert if it consumes more than this upon reception, or if gas costs change in the future. See https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/.
Note the comment Send ETH or ERC20 token from this contract to an external contract, so it is indeed the intention to send ETH to a contract.

Tools Used

Code inspection

Recommended Mitigation Steps

Use call() instead, checking the return value and protecting against reentrancy.

[c4-judge]

dmvt marked the issue as duplicate of #14

[c4-judge]

dmvt marked the issue as satisfactory

[C4-Staff]

liveactionllama marked the issue as duplicate of #369