Lost funds over a misplaced value

[code423n4]

Lines of code

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

Vulnerability details

Impact

In DebtDao, a credit line can be created over any whitelisted erc20 token or ETH. A mutual agreement, between the borrower and the lender, establishes the parameters with which a line is created. Whenever the agreement is over a token, the balance and amounts (owed and due) are managed in accordance with the token's implementation. But if the agreement is over ETH, there's a potential mismatch between balances and the amounts registered using the Denominations.ETH placeholder.

Thus, in receiveTokenOrETH, since the concern is to protect the line in amounts received less than expected (if(msg.value < amount) { revert TransferFailed(); }), it allows for potential mistakes whenever an end-user calls addCredit, increaseCredit, depositAndClose, depositAndRepay and close.

External requirements:

• lender or borrower transfers a value greater than the intended amount.

Proof of Concept

Simple scenario where the lender transfers a bigger value than the amount agreed with the borrower. After the borrower pays his dues, the line keeps the extra amount of ether after reaching its end state.

function test_lost_funds_over_misplaced_value() public {
    assertEq(address(line).balance, 0, "Line balance should be 0");
    assertEq(lender.balance, mintAmount, "lender should have initial mint balance");
    deal(borrower, 1 ether);

    console.log("\n1. initial state");
    console.log(address(line).balance); //  0
    console.log(lender.balance);        //  100000000000000000000
    console.log(borrower.balance);      //  1000000000000000000
    
    vm.startPrank(borrower);
    line.addCredit(dRate, fRate, 1 ether, Denominations.ETH, lender);
    vm.stopPrank();

    vm.startPrank(lender);
    line.addCredit{value: 2 ether}(dRate, fRate, 1 ether, Denominations.ETH, lender); //value does not match the agreed amount
    vm.stopPrank();
    bytes32 id = line.ids(0);
    assert(id != bytes32(0)); //confirms ok

    console.log("\n2. agreed credit but misplaced value");
    console.log(address(line).balance); //  2000000000000000000
    console.log(lender.balance);        //  98000000000000000000
    console.log(borrower.balance);      //  1000000000000000000

    vm.startPrank(borrower);
    line.borrow(id, 1 ether / 2);
    vm.stopPrank();

    console.log("\n3. borrower borrows 0.5 ether");
    console.log(address(line).balance); //  1500000000000000000
    console.log(lender.balance);        //  98000000000000000000
    console.log(borrower.balance);      //  1500000000000000000

    vm.startPrank(borrower);  
    line.depositAndClose{value: 1 ether / 2}(); //
    vm.stopPrank();
    
    console.log("\n4. borrower pays back 0.5 ether");
    console.log(address(line).balance); //  1000000000000000000
    console.log(lender.balance);        //  99000000000000000000
    console.log(borrower.balance);      //  1000000000000000000

    console.log("\nend state reached");                   
    console.log(uint(line.status()));           // 3
    console.log(uint(LineLib.STATUS.REPAID));   // 3
}

Tools Used

VSCode, Foundry

Recommended Mitigation Steps

Consider altering the check in L71 to:

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

If for some reason there's a need to keep a margin in the amount received, set a reasonable constant to control it.

[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