Borrower might unexpectedly take an incredibly overcollateralized loan.

Summary

One way to create loans, is by matching proposals via matchProposals.

One of the requirements needed to match two proposals is that the borrowRequest has higher collateralization ratio than the loanOffer

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

Then, when the two proposals are met, the actual collateral used is based on the loanOffer collateral ratio (the smaller one).

            collateralAmountRequired = _calculateCollateralAmountRequired(
                loanOffer,
                loanOfferFulfillment,
                fulfillAmount
            );

Because of this, the borrow's fulfillment will be updated at a lower rate.

Then, when the last fulfiller, accepts that borrow using acceptBorrowRequest, they'll create a significantly overcollateralized loan, because of the following lines of code:

    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;
        }
    }

Root Cause

The possibility of filling a borrow request at lower collateralization rate than expected.

Attack Path

Bob wants to borrow 1000 USDC against their 2000 shares position.
Alice has an active loanOffer, giving 900 USDC and requiring only 1500 shares as collateral
matchProposals is called, filling the entire loanOffer. The unused fulfillment amount in the borrow is 500 shares.
Bob decides to repay early this loan. (e.g. due to bad interest rate, or due to not needing so much capital at that moment)
Another user decides to fulfill the rest of Bob's loan. The loan is created for 100 USDC and the collateral is set to the remaining 2000 - 1500 = 500 collateral. Bob is forced into a loan with 2.5x more collateral than what they would usually expect

Affected Code

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

Impact

Borrower may be forced into significantly undesirable loan conditions.

Mitigation

It would be better to simply always round down the collateral, instead of allowing this situation to happen. In practice, 1 wei less collateral would not make any difference to any lender.

    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;
-        }
    }

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

bughuntoor - Borrower might unexpectedly take an incredibly overcollateralized loan. #221