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

5 stars 4 forks source link

eeyore - Seizing collateral in the `call()` function, or in `seize()` without proper delay after `call()` leads to borrower fund loss. #255

Open sherlock-admin2 opened 1 month ago

sherlock-admin2 commented 1 month ago

eeyore

Medium

Seizing collateral in the call() function, or in seize() without proper delay after call() leads to borrower fund loss.

Summary

When a loan is created, the lender accepts that the collateral used for the loan can become worthless at the time of the condition token price availability.

The protocol decision to not allow creating new loans, auctioning, and refinancing when the condition token price is available is correct, as it cannot be determined that the collateral used is worth anything at that moment.

However, the protocol decision to seize collateral via call() or indirectly in seize() without proper delay leads to borrower fund loss and cannot be seen as a proper decision.

Seizing collateral ahead of time benefits only the lender in cases when the collateral has not become worthless, shifting the already accepted risk from the lender to the borrower and significantly affecting the borrower, with a loss in the form of collateral_worth - borrowed_amount.

Root Cause

The root cause is that the collateral used by the borrower to secure the loan can be worth more than it was before once the question result is known. In a situation where the condition token used as collateral was correct in answering the question, its worth may increase compared to when the loan was created. Seizing this token in the call() function results in a direct loss for the borrower, as they should be allowed to repay their loan within a reasonable time after the call() and not be front-run by the lender, who would extract extra value from the loan.

https://github.com/sherlock-audit/2024-09-predict-fun/blob/main/predict-dot-loan/contracts/PredictDotLoan.sol#L544-L545
https://github.com/sherlock-audit/2024-09-predict-fun/blob/main/predict-dot-loan/contracts/PredictDotLoan.sol#L616

Internal Pre-Conditions

None.

External Pre-Conditions

The price for the condition token used as collateral is available.

Attack Path

Occurs naturally:

  1. Lender calls the call() function when the price is available.

Impact

Borrower fund loss.

PoC

Not needed.

Mitigation

The lender should not be allowed to seize the borrower's collateral without sufficient delay for the borrower to react.

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

            emit LoanCalled(loanId);
-        }
-        if (!_isQuestionPriceAvailable(loan.questionType, positionQuestion[loan.positionId])) {
            if (loan.callTime + AUCTION_DURATION >= block.timestamp) {
                revert AuctionNotOver();
            }
-        }