Use call() rather than transfer() for ETH transfers

[code423n4]

Lines of code

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

Vulnerability details

Impact

The transfer() function only provides a stipend for the recipient to use of 2300 gas. If the recipient uses more than that, transfers will fail. For longevity it is better to use call() rather than transfer() to ensure that future gas increases won’t affect ETH transfers.

The transfer() function can fail for a number of reasons;

1. The claimer smart contract does not implement a payable function.
2. The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit.
3. The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call’s gas usage above 2300.

Additionally, using higher than 2300 gas might be mandatory for some multi-sig wallets.

Recommended mitigation steps

The following git diff demonstrates how call() can be used instead of transfer();

diff --git a/contracts/utils/LineLib.sol b/contracts/utils/LineLib.sol
index fb112cb..97ddbf2 100644
--- a/contracts/utils/LineLib.sol
+++ b/contracts/utils/LineLib.sol
@@ -45,7 +45,8 @@ library LineLib {
     if(token!= Denominations.ETH) { // ERC20
         IERC20(token).safeTransfer(receiver, amount);
     } else { // ETH
-            payable(receiver).transfer(amount);
+            (bool success,) = payable(receiver).call{value: amount}("");
+            require(success, "ETH transfer failed");
     }
     return true;
 }

Note call() introduces the potential for re-entrancy but future proofs ETH transfers into the future in the even that gas usage changes. Functions including call should follow the Check, Effects, Interactions pattern and or Openzeppelin’s ReentrancyGuard.

[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