Use of Transfer Might Render ETH Impossible to Withdraw

[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

When transferring ETH to the receiver, LineLib.sol uses Solidity’s transfer function. This has some notable shortcomings when the recipient is a smart contract as denoted by the comment on Line 29, which can render ETH impossible to transfer.

Proof of Concept

Specifically, the transfer will inevitably fail when the recipient is a smart contract that:

1. does not implement a payable fallback function, or
2. implements a payable fallback function which would incur more than 2300 gas units, or
3. implements a payable fallback function incurring less than 2300 gas units but is called through a proxy that raises the call’s gas usage above 2300.

Tools Used

Manual inspection and read up

Recommended Mitigation Steps

Consider using:

call, which returns a boolean value indicating success or failure, in combination with re-entrancy guard is highly recommended after December 2019.

error TransferFailed();

(bool success, ) = receiver.call{ value: amount }('');
if(!success) revert TransferFailed();

Alternatively, sendValue() available in OpenZeppelin Contract’s Address library can be used to transfer the Ether without being limited to 2300 gas units. And again, the risks of re-entrancy stemming from the use of this function can be mitigated by tightly following the “Check-effects-interactions” pattern and using OpenZeppelin Contract’s ReentrancyGuard contract.

Lastly, whenever token/fund transfer to the recipient from the contract is involved, use a pull-over-push pattern where possible. With a separate withdraw() implemented, the onus is on the users to receive the fund/token properly.

[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