jasonxiale - APR in `OCC_Modular.applyCombine` is not correct

[sherlock-admin4]

jasonxiale

high

APR in OCC_Modular.applyCombine is not correct

Summary

The APR calculation in OCC_Modular.applyCombine is not correctly, which might causes the system loses interest, or the borrower pays more interest than repay the loan one-by-one

Vulnerability Detail

In OCC_Modular.applyCombine, the APR depends on the sum APR of every loans, principalOwed of every loan.

742     function applyCombine(uint256 id) external {
...
759
760         uint256 notional;
761         uint256 APR;
762
763         for (uint256 i = 0; i < combinations[id].loans.length; i++) {
764             uint256 loanID = combinations[id].loans[i];
...
773             notional += loans[loanID].principalOwed;
774             APR += loans[loanID].principalOwed * loans[loanID].APR;
775             loans[loanID].principalOwed = 0;
776             loans[loanID].paymentDueBy = 0;
777             loans[loanID].paymentsRemaining = 0;
778             loans[loanID].state = LoanState.Combined;
779         }
780
781         APR = APR / notional;
782
...
803         loans[loanCounter] = Loan(
804             _msgSender(), notional, APR, APRLateFee, block.timestamp - block.timestamp % 7 days + 9 days + paymentInterval,
805             term, term, paymentInterval, block.timestamp - 1 days, gracePeriod, paymentSchedule, LoanState.Active
806         );
...

But this calcuation is not correctly. Supposed Alice(the borrower) has two loans, both of the two loans are Bullet type: the 1st loan is 1000$, APR=0.15, and the term is 2 years; the 2nd loan is 2000$, APR=0.12, and the term is 1 years;

1. If Alice repays the loans as expected, she will pay 1000 0.15 2 + 2000 0.12 1 = 540$ as interest.
2. If Alice repays the loans after calling applyCombine, the APR will be APR = (1000 0.15 + 2000 0.12) / (1000 + 2000) = 0.13, and the principalOwed of the new combined loan will be 1000 + 2000 = 3000$. 1) So if the loan.term is 1 year, Alice will pay 3000 0.13 = 390$ as interest, the system will loss profit. 2) And if the loan.term is 1.5 years, the interest will be 3000 0.13 * 1.5 = 585$, in such case, Alice will pay more interest than repay the loans one by one

Impact

The APR calculation in OCC_Modular.applyCombine is not correctly, which might causes the system loses interest, or the borrower pays more interest than repay the loan one-by-one

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/d4111645b19a1ad3ccc899bea073b6f19be04ccd/zivoe-core-foundry/src/lockers/OCC/OCC_Modular.sol#L773-L774 https://github.com/sherlock-audit/2024-03-zivoe/blob/d4111645b19a1ad3ccc899bea073b6f19be04ccd/zivoe-core-foundry/src/lockers/OCC/OCC_Modular.sol#L781

Tool used

Manual Review

Recommendation

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, the loss of precision is inevitable in fixed point calculations. This one doesn't cause any impact other than tiny difference in calculations