LendingTerm#getLoanDept() More openingFee is charged to loans that are partially repayed

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L226

Vulnerability details

Impact

openingFee is a fee charged once when opening a loan. When repaying a loan, the getLoanDebt() is called and the openingFee calculations are made and added to the loanDept. It is added every time a user repays which does not seems to be a problem however when a user is partially repaying the accumulated fee is more then if he repayed it at once.

            loanDebt += (borrowAmount * _openingFee) / 1e18;

If we assume a user first partial repay his loan, in the _partialRepay() a % of the repaid amount is transferred for the protocol for the fee. After time if he partial repay again, the getLoanDebt() calculates again the openingFee for the new borrowAmount(as it was decreased from the first repay), however more fee is calculated than expected.

Proof of Concept

Paste the following test inside test/unit/loan/LendingTerm.t.sol.

function testMoreOpeningFeeCharged() public {
        LendingTerm term2 = LendingTerm(
            Clones.clone(address(new LendingTerm()))
        );
        term2.initialize(
            address(core),
            term.getReferences(),
            LendingTerm.LendingTermParams({
                collateralToken: address(collateral),
                maxDebtPerCollateralToken: _CREDIT_PER_COLLATERAL_TOKEN,
                interestRate: 0,
                maxDelayBetweenPartialRepay: _MAX_DELAY_BETWEEN_PARTIAL_REPAY,
                minPartialRepayPercent: _MIN_PARTIAL_REPAY_PERCENT,
                openingFee: 0.05e18,
                hardCap: _HARDCAP
            })
        );
        vm.label(address(term2), "term2");
        guild.addGauge(1, address(term2));
        guild.decrementGauge(address(term), _HARDCAP);
        guild.incrementGauge(address(term2), _HARDCAP);
        vm.startPrank(governor);
        core.grantRole(CoreRoles.RATE_LIMITED_CREDIT_MINTER, address(term2));
        core.grantRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(term2));
        vm.stopPrank();

        // prepare & borrow
        uint256 borrowAmount = 20_000e18;
        uint256 collateralAmount = 12e18;
        collateral.mint(address(this), collateralAmount);
        collateral.approve(address(term2), collateralAmount);
        bytes32 loanId = term2.borrow(borrowAmount, collateralAmount);

        vm.warp(block.timestamp + 10);
        vm.roll(block.number + 1);

        assertEq(term2.getLoanDebt(loanId), 21_000e18);

        // it should be 21 000e18
        uint256 firstLoanDeptBefore = term2.getLoanDebt(loanId);
        console.log("firstLoanDeptBefore:", firstLoanDeptBefore);

        credit.mint(address(this), 5_000e18);
        credit.approve(address(term2), 5_000e18);

        term2.partialRepay(loanId, 5_000e18);

        // it should be 16000000000000000000000 however we can see it the console log it is 16000000000000000005000
        uint256 loanDeptAfterPartialRepay = term2.getLoanDebt(loanId);
        console.log("loanDeptAfterPartialRepay:", loanDeptAfterPartialRepay);

        credit.mint(address(this), 5_000e18);
        credit.approve(address(term2), 5_000e18);

        term2.partialRepay(loanId, 5_000e18);

        // it should be 11000000000000000000000 however we can see it the console log it is 11000000000000000019438
        uint256 loanDeptAfterSecondPartialRepay = term2.getLoanDebt(loanId);
        console.log("loanDeptAfterSecondPartialRepay:", loanDeptAfterSecondPartialRepay);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Charge the opening fee inside the borrow() when opening a loan.

Assessed type

Other

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #700

[c4-judge]

Trumpero marked the issue as unsatisfactory: Invalid

[0xSilvermist]

Hi @Trumpero I think this issue was wrongly duplicated with #700. I respectfully disagree that this issue is invalid since it was duplicated with an issue that shows that the openingFee is charged multiple times. However, this particular issue highlights that an excess amount of openingFee is being charged, rather than multiple times which can be seen from the PoC.

Here is an another example: Term with 0% APR, 5% open fee

• Borrow of 1000
• getLoanDebt() returns 1050 (50 openingFee)
• there is a partialRepay of 500
• percentRepaid = 47%
• principalRepaid = 1000 * 47% = 470
• interestRepaid = 500 - 470 = 30

After repayment, loan principal = 530, and the getLoanDebt() should return 550 because 530 is the principle and since we pay 30 openingFee with the first partialRepay, there must be only 20 openingFee left. However, the getLoanDebt() returns 556.5.

[Trumpero]

@MariinaKP I believe your calculation is incorrect because of rounding down. In the above case:

• percentRepaid = 47,6%
• principalRepaid = 476
• interestRepaid = 24

After repayment, loan pricipal = 524 -> opening fee left = 26 -> total opening fee is still 50

It can be easily proven by the code that the total principal repaid of a loan equals the initial borrowed amount. Since the borrowed amount decreases with each principal repayment, the total opening fee remains unchanged.