Skip to content

Latest commit

 

History

History
50 lines (32 loc) · 2.77 KB

H-07.md

File metadata and controls

50 lines (32 loc) · 2.77 KB
Raw

Mutual consent may lead to ETH deposit lost in addCredit function of LineOfCredit contract

The addCredit function requires the consent of both lender and borrower to proceed and create the credit. If ETH is used as the token and the lender consents first, then the borrower won't be able to give consent, preventing the credit from being created and losing the deposited ETH.

Impact

When ETH is used as the credit token and if the lender is the first to give consent, he will execute a call to addCredit with a transaction value of the desired deposit amount of the credit.

Since he is the first to give consent, the mutualConsent modifier will wait for the other party (borrower) before continuing execution, however the ETH has been successfully transferred from the lender to the line contract.

Now, if the borrower tries to give consent to this addCredit transaction, the body of the function will be executed (since consent has been given by both parties) but it will get an error because LineLib.receiveTokenOrETH will require the borrower transaction to include the ETH payment.

This will effectively prevent the addCredit call from completing, and the lender will lose his deposit.

A different but similar issue happens if the borrower never consents, the lender has already transferred the ETH but there's no way to "cancel" that consent to refund the payment.

PoC

Lender wants to create a new credit and goes first, transferring 1 ETH. Then the borrower tries to give consent with the same params, but the call fails since it will ask the borrower for the ETH payment.

Note: the context for this test (setup, variables and helper functions) is similar to the one found in the file LineOfCredit.t.sol.

function test_addCredit_BadMutualConsent() public {
    address lender = makeAddr("lender");
    _mintAndApprove(lender);

    uint256 amount = 1 ether;
    uint256 lenderBalanceBefore = lender.balance;

    // Lender gives consent first
    vm.prank(lender);
    line.addCredit{value: amount}(dRate, fRate, amount, Denominations.ETH, lender);

    // lender has already sent 1 ETH
    assertEq(lender.balance, lenderBalanceBefore - amount);

    // borrower tries to consent the credit but this will fail since addCredit will ask THE BORROWER to send the ETH value
    vm.expectRevert(LineLib.TransferFailed.selector);
    vm.prank(borrower);
    line.addCredit(dRate, fRate, amount, Denominations.ETH, lender);

    // We end up in an inconsistent state, borrower won't be able to give consent and the lender has already lost the ETH
}

Recommendation

  1. Make sure the borrower side gives consent first. or...
  2. Instead of using native ETH (or the equivalent in the chain this gets deployed) use the wrapped ERC20 version WETH. This will enable pull payments for ETH.