USE OF PAYABLE.TRANSFER() MAY LOCK USER FUNDS

[code423n4]

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L48

Vulnerability details

Impact

The use of payable.transfer() is heavily frowned upon because it can lead to the locking of funds. The transfer() call requires that the recipient has a payable callback, only provides 2300 gas for its operation.
This means the following cases can cause the transfer to fail:

• The contract does not have a payable callback
• The contract’s payable callback spends more than 2300 gas (which is only enough to emit something)
• The contract is called through a proxy which itself uses up the 2300 gas

If a user falls into one of the above categories, they’ll be unable to receive funds. Inaccessible funds means loss of funds, which is Medium severity.

Proof of Concept

The LineLib.sol sendOutTokenOrETH is being use extensively from other contracts, as this is kind of a library for sending token or ETH from the contract to other receiver contract.

File: LineLib.sol
34:     function sendOutTokenOrETH(
35:       address token,
36:       address receiver,
37:       uint256 amount
38:     )
39:       external
40:       returns (bool)
41:     {
42:         if(token == address(0)) { revert TransferFailed(); }
43:         
44:         // both branches revert if call failed
45:         if(token!= Denominations.ETH) { // ERC20
46:             IERC20(token).safeTransfer(receiver, amount);
47:         } else { // ETH
48:             payable(receiver).transfer(amount);
49:         }
50:         return true;
51:     }

Tools Used

Manual analysis

Recommended Mitigation Steps

Use address.call{value:x}() instead.

[c4-judge]

dmvt marked the issue as duplicate of #14

[c4-judge]

dmvt marked the issue as partial-50

[c4-judge]

dmvt marked the issue as satisfactory

[C4-Staff]

liveactionllama marked the issue as duplicate of #369