sherlock-audit / 2024-09-predict-fun-judging

5 stars 4 forks source link

nikhilx0111 - a malicious lender can sell already repaid loan #283

Open sherlock-admin3 opened 1 month ago

sherlock-admin3 commented 1 month ago

nikhilx0111

High

a malicious lender can sell already repaid loan

Summary

When a new loan is created, the status of the loan is set to loans[id].status = LoanStatus.Active. However, when a borrower repays a loan, the status is not reset when a lender wants to initiate a auction calls the function call

function call(uint256 loanId) external nonReentrant {
    Loan storage loan = loans[loanId];

    _assertAuthorizedCaller(loan.lender);
    _assertLoanStatus(loan.status, LoanStatus.Active);

    if (loan.startTime + loan.minimumDuration > block.timestamp) {
        revert LoanNotMatured();
    }

    if (_isQuestionPriceAvailable(loan.questionType, positionQuestion[loan.positionId])) {
        _seize(loanId, loan);
    } else {
        loan.status = LoanStatus.Called;
        loan.callTime = block.timestamp;

        emit LoanCalled(loanId);

the code checks if the status is active and if the loan has matured but theres no check if the loan is repaid now when we see the repay function

 function repay(uint256 loanId) external nonReentrant {
    Loan storage loan = loans[loanId];

    _assertAuthorizedCaller(loan.borrower);

    LoanStatus status = loan.status;
    if (status != LoanStatus.Active) {
        if (status != LoanStatus.Called) {
            revert InvalidLoanStatus();
        }
    }

    uint256 debt = _calculateDebt(loan.loanAmount, loan.interestRatePerSecond, _calculateLoanTimeElapsed(loan));

    loan.status = LoanStatus.Repaid;

    LOAN_TOKEN.safeTransferFrom(msg.sender, loan.lender, debt);
    CTF.safeTransferFrom(address(this), msg.sender, loan.positionId, loan.collateralAmount, "");

    emit LoanRepaid(loanId, debt);

The function only sets the status to "repaid" but doesn't reset the active status of the loan. As a result, a malicious user can call the auction on an empty loan and exploit another lender who will buy the empty loan, even in the auction function theres is no check if the loan is repaid the function only checks if the loan is active which will be true since the status is never reset

function auction(uint256 loanId) external nonReentrant whenNotPaused {
    Loan storage loan = loans[loanId];

    _assertLoanStatus(loan.status, LoanStatus.Called);

    _assertLenderIsNotBorrower(msg.sender, loan.borrower);

    _assertNewLenderIsNotTheSameAsOldLender(msg.sender, loan.lender);

Root Cause

https://github.com/sherlock-audit/2024-09-predict-fun/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L454-L473

https://github.com/sherlock-audit/2024-09-predict-fun/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L534-L550

https://github.com/sherlock-audit/2024-09-predict-fun/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L561-L568

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

loss of funds for innocent lenders who will buy the empty loan

PoC

No response

Mitigation

reset the status of the loan when the loan is repaid