The last lender of a partially fulfilled borrow request might have a significantly higher collateral ratio than the collateral ratio specified in the borrow request

Summary

The last lender of a partially fulfilled borrow request might have a significantly higher collateral ratio than the collateral ratio specified in the borrow request.

Root Cause

In _acceptOffer, the collateralAmountRequired is the leftover collateral when the borrow request is fully filled

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

    function _calculateCollateralAmountRequired(
        Proposal calldata proposal,
        Fulfillment storage fulfillment,
        uint256 fulfillAmount
    ) private view returns (uint256 collateralAmountRequired) {
        if (fulfillment.loanAmount + fulfillAmount == proposal.loanAmount) {
>>          collateralAmountRequired = proposal.collateralAmount - fulfillment.collateralAmount;
        } else {
            collateralAmountRequired = (proposal.collateralAmount * fulfillAmount) / proposal.loanAmount;
        }
    }

The borrower can use matchProposals to match their borrow request to a better loan offer, which has a lower collateral ratio than the ratio of the borrow request

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

        if (
            borrowRequest.collateralAmount * loanOffer.loanAmount <
            borrowRequest.loanAmount * loanOffer.collateralAmount
        ) {
            revert UnacceptableCollateralizationRatio();
        }

then the collateral ratio of the loan is the collateral ratio of the loan offer

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

If the borrower has used their borrow request to match with the lower collateral ratio loan offer, then the last lender that fully accepts the borrow request will have a significantly higher collateral ratio than the collateral ratio specified in the borrow request.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Alice signs a borrow request that has collateralAmount = 20 ether, and loanAmount = 10 ether (collateral ratio = 200%)
Bob signs a loan offer that has collateralAmount = 5 ether, and loanAmount = 5 ether (collateral ratio = 100%)
Since Bob's loan offer has a lower collateral ratio than her borrow request, the loan offer is better for her. Alice matches Bob's loan offer against her. Current states:
- fulfillment.collateralAmount = 15 ether
- fulfillment.loanAmount = 5 ether
Cindy fully accepts Alice's borrow request, and Cindy benefits from a loan with high collateral ratio (15 ether / 5 ether = 300%)

We believe the loan should only have a collateral ratio lower than or equal to 200%.

Impact

The last lender (Cindy) might have a significantly higher collateral ratio than the collateral ratio specified in the borrow request
The borrower (Alice) will have a loan that has a higher collateral ratio than expected.

PoC

Add a view function in PredictDotLoan to check the collateral ratio of a loan

contract PredictDotLoan is AccessControl, EIP712, ERC1155Holder, IPredictDotLoan, Pausable, ReentrancyGuard {
    ...
    function getLoanCollateralRatio(uint256 loanId) public view returns (uint256) {
        IPredictDotLoan.Loan memory loan = loans[loanId];
        return loan.collateralAmount * 1 ether / loan.loanAmount;
    }
}

Run command: forge test --match-path test/foundry/PoC.t.sol -vv

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.25;

import {IPredictDotLoan} from "../../contracts/interfaces/IPredictDotLoan.sol";
import {TestHelpers} from "./TestHelpers.sol";
import {console} from "forge-std/Test.sol";

contract PoC is TestHelpers {
    uint256 aliceKey = 1;
    uint256 bobKey = 2;

    address alice = vm.addr(aliceKey);
    address bob = vm.addr(bobKey);
    address cindy = makeAddr('cindy');

    function setUp() public {
        _deploy();

        vm.prank(alice);
        mockCTF.setApprovalForAll(address(predictDotLoan), true);
        _mintCTF(alice);

        mockERC20.mint(bob, LOAN_AMOUNT);
        vm.prank(bob);
        mockERC20.approve(address(predictDotLoan), LOAN_AMOUNT);

        mockERC20.mint(cindy, LOAN_AMOUNT);
        vm.prank(cindy);
        mockERC20.approve(address(predictDotLoan), LOAN_AMOUNT);
    }

    function test_PoC() public {
        // Collateral ratio: 20 / 10 = 200%
        IPredictDotLoan.Proposal memory borrowRequest = _generateBorrowRequest(
            IPredictDotLoan.QuestionType.Binary,
            alice,
            aliceKey,
            20 ether,
            10 ether
        );

        // Collateral ratio: 5 / 5 = 100%
        IPredictDotLoan.Proposal memory loanOffer = _generateLoanOffer(
            IPredictDotLoan.QuestionType.Binary,
            bob,
            bobKey,
            5 ether,
            5 ether
        );

        predictDotLoan.matchProposals(borrowRequest, loanOffer);

        vm.prank(cindy);
        predictDotLoan.acceptBorrowRequest(borrowRequest, 5 ether);

        console.log("First loan's collateral ratio: %e", predictDotLoan.getLoanCollateralRatio(1));
        console.log("Second loan's collateral ratio: %e", predictDotLoan.getLoanCollateralRatio(2));
    }

    function _generateBorrowRequest(
        IPredictDotLoan.QuestionType questionType,
        address from,
        uint256 privateKey,
        uint256 collateralAmount,
        uint256 loanAmount
    ) internal view returns (IPredictDotLoan.Proposal memory proposal) {
        proposal = _generateBaseProposal(questionType);
        proposal.collateralAmount = collateralAmount;
        proposal.loanAmount = loanAmount;
        proposal.from = from;
        proposal.proposalType = IPredictDotLoan.ProposalType.BorrowRequest;

        (, uint128 borrowingNonce) = predictDotLoan.nonces(from);
        proposal.nonce = borrowingNonce;

        proposal.signature = _signProposal(proposal, privateKey);
    }

    function _generateLoanOffer(
        IPredictDotLoan.QuestionType questionType,
        address from,
        uint256 privateKey,
        uint256 collateralAmount,
        uint256 loanAmount
    ) internal view returns (IPredictDotLoan.Proposal memory proposal) {
        proposal = _generateBaseProposal(questionType);
        proposal.collateralAmount = collateralAmount;
        proposal.loanAmount = loanAmount;
        proposal.from = from;
        proposal.proposalType = IPredictDotLoan.ProposalType.LoanOffer;

        (uint128 lendingNonce, ) = predictDotLoan.nonces(from);
        proposal.nonce = lendingNonce;

        proposal.signature = _signProposal(proposal, privateKey);
    }
}

Logs:

  First loan's collateral ratio: 1e18
  Second loan's collateral ratio: 3e18

Mitigation

In _calculateCollateralAmountRequired, the collateralAmountRequired is the leftover collateral only when the leftover amounts are only a few weis.

    function _calculateCollateralAmountRequired(
        Proposal calldata proposal,
        Fulfillment storage fulfillment,
        uint256 fulfillAmount
    ) private view returns (uint256 collateralAmountRequired) {
    collateralAmountRequired = (proposal.collateralAmount * fulfillAmount) / proposal.loanAmount;

    if (fulfillment.loanAmount + fulfillAmount == proposal.loanAmount && proposal.collateralAmount - fulfillment.collateralAmount - collateralAmountRequired < THRESHOLD) {
            collateralAmountRequired = proposal.collateralAmount - fulfillment.collateralAmount;
        } 
    }

THRESHOLD could be 10.

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

iamnmt - The last lender of a partially fulfilled borrow request might have a significantly higher collateral ratio than the collateral ratio specified in the borrow request #125