056Security - Discrepency between protocol documentation and code implementation for `matchProposals(...)`

[sherlock-admin4]

056Security

Medium

Discrepency between protocol documentation and code implementation for matchProposals(...)

Summary

The predict.fun protocol has written a very well-documented README.md file where they explain how each user interaction should function. The code strictly follows the shown diagrams and explanations, however, there is a mismatch between the documentation requirements for matchProposals(...) and what is present in the code.

Root Cause

If we take a look at what the documentation specifies about the collateralization ratios and durations of the borrow and loan proposals, we can see:

In order for a loan offer to be matched with a borrow request, the following conditions must be fulfilled:

1. The borrow request's collateralization ratio must be higher than the loan offer's collateralization ratio
2. The borrow request's interest rate per second must be higher than the loan offer's interest rate per second
3. The loan offer's duration must be higher than the borrow request's duration

However the code implies that the borrowRequest.interestRatePerSecond could equal the loanOffer.interestRatePerSecond as it is written here:

if (borrowRequest.interestRatePerSecond < loanOffer.interestRatePerSecond) {
    revert UnacceptableInterestRatePerSecond();
}

The same is done with the duration - borrowRequest.duration can equal loanOffer.duration here

 if (borrowRequest.duration > loanOffer.duration) {
    revert UnacceptableDuration();
}

If we take a look at the README we can see that whenever a <=/>= comparison is needed, the docs mention it:

Refinance a loan In order for a new loan to be refinanced, the following conditions must be fulfilled:

1. The new loan offer must have an interest rate at least as good as the current loan's
2. The new loan offer's collateral amount required must not be higher than the current loan's

Internal pre-conditions

N/A

External pre-conditions

N/A

Attack Path

N/A

Impact

Discrepancy between spec and code.

PoC

N/A

Mitigation

--- a/predict-dot-loan/contracts/PredictDotLoan.sol
+++ b/predict-dot-loan/contracts/PredictDotLoan.sol
@@ -337,11 +337,11 @@ contract PredictDotLoan is AccessControl, EIP712, ERC1155Holder, IPredictDotLoan
         _assertValidInterestRatePerSecond(loanOffer.interestRatePerSecond);
         _assertValidInterestRatePerSecond(borrowRequest.interestRatePerSecond);

-        if (borrowRequest.interestRatePerSecond < loanOffer.interestRatePerSecond) {
+        if (borrowRequest.interestRatePerSecond <= loanOffer.interestRatePerSecond) {
             revert UnacceptableInterestRatePerSecond();
         }

-        if (borrowRequest.duration > loanOffer.duration) {
+        if (borrowRequest.duration => loanOffer.duration) {
             revert UnacceptableDuration();
         }