Skip to content
This repository has been archived by the owner on Oct 27, 2024. It is now read-only.

saidam017 - Borrowers could skip at least one period of interest payment when paying off the loan in full and end up paying less interest #452

Closed
sherlock-admin3 opened this issue Apr 25, 2024 · 1 comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin3
Copy link

sherlock-admin3 commented Apr 25, 2024

saidam017

high

Borrowers could skip at least one period of interest payment when paying off the loan in full and end up paying less interest

Summary

When borrowers call callLoan to pay off the loan in full, it will calculate the interest for one payment interval and late fee (if applicable). However, this doesn't take into account whether the users have paid the interest for the previous period, especially with terms that have a longer grace period, causing users to pay less interest than they should.

Vulnerability Detail

When users trigger callsLoan, it will calculate the interest for one payment interval and late fee. Then users have to pay the interest, late fee and the remaining principal to the protocol.

https://github.com/sherlock-audit/2024-03-zivoe/blob/main/zivoe-core-foundry/src/lockers/OCC/OCC_Modular.sol#L492-L518

    function callLoan(uint256 id) external nonReentrant {
        require(
            _msgSender() == loans[id].borrower || IZivoeGlobals_OCC(GBL).isLocker(_msgSender()), 
            "OCC_Modular::callLoan() _msgSender() != loans[id].borrower && !isLocker(_msgSender())"
        );
        require(loans[id].state == LoanState.Active, "OCC_Modular::callLoan() loans[id].state != LoanState.Active");

        uint256 principalOwed = loans[id].principalOwed;
>>>     (, uint256 interestOwed, uint256 lateFee,) = amountOwed(id);

        emit LoanCalled(id, principalOwed + interestOwed + lateFee, principalOwed, interestOwed, lateFee);

        // Transfer interest to YDL if in same format, otherwise keep here for 1INCH forwarding.
        if (stablecoin == IZivoeYDL_OCC(IZivoeGlobals_OCC(GBL).YDL()).distributedAsset()) {
            IERC20(stablecoin).safeTransferFrom(_msgSender(), IZivoeGlobals_OCC(GBL).YDL(), interestOwed + lateFee);
        }
        else {
            IERC20(stablecoin).safeTransferFrom(_msgSender(), OCT_YDL, interestOwed + lateFee);
        }

        IERC20(stablecoin).safeTransferFrom(_msgSender(), owner(), principalOwed);

        loans[id].principalOwed = 0;
        loans[id].paymentDueBy = 0;
        loans[id].paymentsRemaining = 0;
        loans[id].state = LoanState.Repaid;
    }

Consider the scenario when a loan configured with payment interval of 7 days and grace period of 7 days. If a borrower misses one payment, they have 7 days of grace period to pay the previous interest payment. However, if the borrower waits until nearly the end of the grace period and then triggers the callLoan function to fully pay off the loan, they only need to pay for one interest period plus the late fee, instead of two interest periods plus the late fee.

Impact

This situation can become problematic, especially if the late fee for 7 days is lower than the interest for one period, as it can be abused to pay less to the protocol than should be paid.

It have more severe impact if the grace period may be longer than the payment interval, and the borrower may miss several loan payments.

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/main/zivoe-core-foundry/src/lockers/OCC/OCC_Modular.sol#L492-L518

Tool used

Manual Review

Recommendation

Consider tracking how much interest payment is skipped by the borrower and ensure that all of the interest is charged when callLoan is called.

Duplicate of #97

@github-actions github-actions bot closed this as completed May 5, 2024
@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 5, 2024
@sherlock-admin2
Copy link
Contributor

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

panprog commented:

medium, dup of #97, if there are more than 1 interest payments missed by borrower, then callLoan takes only 1 period payment, allowing borrower to skip paying the other periods interest and lateFee payments.

@sherlock-admin2 sherlock-admin2 changed the title Great Metal Ram - Borrowers could skip at least one period of interest payment when paying off the loan in full and end up paying less interest saidam017 - Borrowers could skip at least one period of interest payment when paying off the loan in full and end up paying less interest May 11, 2024
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label May 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants