Lender or borrower can loose money if mistakenly sends more than the amount of ETH than the amount money

[code423n4]

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L265
https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L223
https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L315
https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/LineLib.sol#L71

Vulnerability details

Impact

Lender or borrower can loose money if mistakenly sends more than the amount of ETH than the amount money

In the design of the protocol, the lender can use the function increaseCredit or addCredit to add the deposit money for the credit.
Or the borrower can use the function depositAndRepay to deposit and repay for the credit

function addCredit(
        uint128 drate,
        uint128 frate,
        uint256 amount,
        address token,
        address lender
    )
        external
        payable
        override
        whileActive
        mutualConsent(lender, borrower)
        returns (bytes32)
    {
      ...
     
      LineLib.receiveTokenOrETH(token, lender, amount);
      ...
    }

function increaseCredit(bytes32 id, uint256 amount)
      external
      payable
      override
      whileActive
      mutualConsentById(id)
      returns (bool)
    {
      ...
     
     LineLib.receiveTokenOrETH(credit.token, credit.lender, amount);
      ...
    }

function depositAndRepay(uint256 amount)
        external
        payable
        override
        whileBorrowing
        returns (bool)
    {
      ...
      LineLib.receiveTokenOrETH(credit.token, msg.sender, amount);
      ...
    }

All these above functions use the amount as input parameter. So after the borrower and lender reaches agreement on the amount, the lender can call increaseCredit or addCredit.
and the borrower can use depositAndRepay always to repay.

But the function LineLib.receiveTokenOrETH(credit.token, msg.sender, amount) just check if the msg.value < amount.

function receiveTokenOrETH(
      address token,
      address sender,
      uint256 amount
    )
      external
      returns (bool)
    {
        if(token == address(0)) { revert TransferFailed(); }
        if(token != Denominations.ETH) { // ERC20
            IERC20(token).safeTransferFrom(sender, address(this), amount);
        } else { // ETH
            if(msg.value < amount) { revert TransferFailed(); }
        }
        return true;
    }

So in case of using ETH, msg.value can be > amount then the transaction still success. But in this case, the lender or the borrower loose money, because the excessed amount (msg.value - amount) is not tracked anywhere in the protocol.

Proof of Concept

The following scenarios is possible and cause users loose money

Scenario 1:

Step 0: The borrower call addCredit or increaseCredit with amount
Step 1: The lender call addCredit or increaseCredit with amount but the msg.value is > amount
The transaction success. The lender lost some money that is (msg.value - amount).

Scenario 2:

Step 0: The credit was created. the borrower has borrowed some money.
Step 1: The borrower call depositAndRepay and sent the msg.value > amount . Then the borrower lost money (msg.value - amount).

Tools Used

VSCode

Recommended Mitigation Steps

Should check

if(msg.value != amount) { revert TransferFailed(); }

[c4-judge]

dmvt marked the issue as duplicate of #25

[c4-judge]

dmvt marked the issue as satisfactory

[C4-Staff]

liveactionllama marked the issue as duplicate of #39