Loans with a minimum borrowing amount can not be partially repaid

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L527-L531

Vulnerability details

Impact

Loans with a minimum borrowing amount can not be partially repaid.

The main reason a borrower wants to partially repay his loan is to prevent his position from going underwater due to accruing interest. The lending terms require a minimum percentage to cover the debt during partial repayment. Also, when a borrower creates a new loan, there is a restriction on the minimum credit amount that can be borrowed. The restriction for the minimum borrow amount is to ensure that the gas costs of liquidation do not outsize the minimum overcollateralization.

// check that enough CREDIT is borrowed
require(
    borrowAmount >= ProfitManager(refs.profitManager).minBorrow(),
    "LendingTerm: borrow amount too low"
);

When a borrower creates a loan with borrowAmount == ProfitManager(refs.profitManager).minBorrow(), it is not possible for him to partially repay his debt. The problem arises from this require statement:

 require(
    borrowAmount - issuanceDecrease >
        ProfitManager(refs.profitManager).minBorrow(),
    "LendingTerm: below min borrow"
);

In this scenario borrowAmount == ProfitManager(refs.profitManager).minBorrow() and the require statement will never be true. Additionally, loans with a borrow amount close to minBorrow can not be partially repaid because uint256 issuanceDecrease = (borrowAmount * percentRepaid) / 1e18;, where percentRepaid = (debtToRepay * 1e18) / loanDebt. In the best-case scenario for the borrower, he will repay the minimum partial repayment percentage from the borrowed amount.

Every newly created loan with a borrowAmount close to the minBorrowAmount can not be partially repaid. Lenders can not prevent their position from going underwater because they can not partially repay their loans as expected at the beginning of the loan creation.

Proof of Concept

Paste the test in LendingTerm.t.sol

 function testLoansWithMinimumBorrowAmountCanNotBePartialRepaid() public {
        /* 
        LendingTerm params:
            maxDebtPerCollateralToken = 2000e18;     -> 2000, same decimals
            interestRate = 0.10e18;                  -> 10% APR
            maxDelayBetweenPartialRepay = 63115200;  -> 2 years
            minPartialRepayPercent = 0.2e18;         -> 20%
            openingFee = 0;                          -> 0%
        */

        // prepare & borrow
        uint256 minBorrowAmount = profitManager.minBorrow(); // borrowAmount
        uint256 collateralAmount = 5e16;

        collateral.mint(address(this), collateralAmount);
        collateral.approve(address(term), collateralAmount);
        bytes32 loanId = term.borrow(minBorrowAmount, collateralAmount);
        assertEq(term.getLoan(loanId).collateralAmount, collateralAmount);

        // partialRepay
        vm.warp(block.timestamp + term.YEAR());
        vm.roll(block.number + 1);

        // The debt is 110e18 after 1 year.
        assertEq(term.getLoanDebt(loanId), 110e18);
       
        // User pay minimum partial repay percent which is 20%.
        credit.mint(address(this), 22e18);
        credit.approve(address(term), 22e18);

        // The transaction revert because
        // (borrowAmount - issuanceDecrease) is lower than minBorrow.
        vm.expectRevert("LendingTerm: below min borrow");
        term.partialRepay(loanId, 24e18);

        //MIN_BORROW increases when creditMultiplier decreases - partialRepay()
    }

Tools Used

Mannual Review

Recommended Mitigation Steps

Pre-calculate the minimum borrow amount. It should be bigger than ProfitManager(refs.profit Manager).minBorrow().

Assessed type

Invalid Validation

[0xSorryNotSorry]

The issue is well demonstrated, properly formatted, contains a coded POC.
Marking as HQ.

[c4-pre-sort]

0xSorryNotSorry marked the issue as high quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[eswak]

Acknowledging this, this is fine IMO because at worst, the loan will miss a partial payment and will be called, it can't be decreased below minBorrow size and should be safe to send into liquidation. We could add a warning on the front-end but this seems like a pretty rare edge case. Disagree with severity because no user funds are at risk and it doesn't prevent the protocol from functioning properly, should be informational imo.

[c4-sponsor]

eswak (sponsor) acknowledged

[c4-sponsor]

eswak marked the issue as disagree with severity

[Trumpero]

Agree with sponsor that this issue should be a QA/info

[c4-judge]

Trumpero changed the severity to QA (Quality Assurance)

[c4-judge]

Trumpero marked the issue as grade-b

[c4-judge]

Trumpero marked the issue as grade-c

[kazantseff]

Hey @Trumpero, I want to understand the difference between this issue and #756 . In both cases the functionality of the protocol is impacted as borrower won't be able to partialy repay his loan, so the impact is the same while the root cause is different. So the question is why this deemed unsatisfactory while #756 is of medium severity? Thanks you!

[Trumpero]

@kazantseff
This issue and #756 have different root causes and impacts. In this issue, users can't partially repay if the remaining borrow amount is too small (below minBorrow). It's not a problem because when the borrow amount is too small, it's an intended design to not allow dividing it smaller. Therefore, users should be aware of it and repay fully for small loans. However, issue #756 represents the inability of partial repayment even in normal conditions and huge loans. Partial repayment will be inactive or bricked when a lending term has a small interest rate (near or equal to 0%), or users repay too quickly. So, #756 has a medium severity impact, while this issue should be a QA/info.