128.md

Faithful Parchment Salmon

Medium

Incorrect interestRatePerSecond check in PredictDotLoan::matchProposals

Summary

The invariant states that the check on line 340 has to revert when borrowRequest.interestRatePerSecond <= loanOffer.interestRatePerSecond however the check only checks for when the condition is less than

Root Cause

In Predictloan:340 the check does not take into consideration == conditions

Internal pre-conditions

none

External pre-conditions

none

Attack Path

Whenever there is a call to match proposals in which (borrowRequest.interestRatePerSecond == loanOffer.interestRatePerSecond) the check will pass

Impact

This breaks the invariant in the code documentation The borrow request's interest rate per second must be higher than the loan offer's interest rate per second and this can further lead to a loss for lenders

PoC

in the match proposal readme we will find this invariant there The borrow request's interest rate per second must be higher than the loan offer's interest rate per second which means whenever the borrow requests interests rate per second is less than or equal to the loan offer's interest rate per second it has to revert

but from the code we see below it does not do that because the check does not take into account whenever the condition is equal

function matchProposals(
        Proposal calldata borrowRequest,
        Proposal calldata loanOffer
    ) external nonReentrant whenNotPaused {
        _assertProposalIsBorrowRequest(borrowRequest);
        _assertProposalIsLoanOffer(loanOffer);

        _assertLenderIsNotBorrower(loanOffer.from, borrowRequest.from);

        uint256 positionId = _derivePositionId(borrowRequest);
        // This also indirectly checks that the questionType is the same
        if (positionId != _derivePositionId(loanOffer)) {
            revert PositionIdMismatch();
        }

        _assertPositionTradeableOnExchange(positionId, borrowRequest.questionType);

        _assertValidInterestRatePerSecond(loanOffer.interestRatePerSecond);
        _assertValidInterestRatePerSecond(borrowRequest.interestRatePerSecond);

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

   ...

}

Mitigation

make sure checks take into consideration situations in which borrow request's interest rate per second is equal to the loan offer's interest rate per second

    function matchProposals(
        Proposal calldata borrowRequest,
        Proposal calldata loanOffer
    ) external nonReentrant whenNotPaused {
        _assertProposalIsBorrowRequest(borrowRequest);
        _assertProposalIsLoanOffer(loanOffer);

        _assertLenderIsNotBorrower(loanOffer.from, borrowRequest.from);

        uint256 positionId = _derivePositionId(borrowRequest);
        // This also indirectly checks that the questionType is the same
        if (positionId != _derivePositionId(loanOffer)) {
            revert PositionIdMismatch();
        }

        _assertPositionTradeableOnExchange(positionId, borrowRequest.questionType);

        _assertValidInterestRatePerSecond(loanOffer.interestRatePerSecond);
        _assertValidInterestRatePerSecond(borrowRequest.interestRatePerSecond);

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

   ...

}