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

0 stars 0 forks source link

Mammoth Basil Baboon - Refinance and Auto-Refinance can be DoS-ed due to collateralization cap #260

Open sherlock-admin4 opened 2 days ago

sherlock-admin4 commented 2 days ago

Mammoth Basil Baboon

Medium

Refinance and Auto-Refinance can be DoS-ed due to collateralization cap

Summary

The predict.fun protocol allows users to seamlessly loan and borrow tokens by creating loan/borrow proposals, which are to be fulfilled. The protocol also includes a refinance function, where borrowers can accept new loan proposals to pay their current debt and, in the long run, get more favorable loan conditions. The protocol has also created an automated refinancing functionality, where when a user opts in, his/her borrows can be refinanced multiple times. As discussed with the development team, these features should be easily used and used as often as needed.

Screenshot 2024-10-07 at 14 23 26

However, due to how each refinanced debt incurs fees (meaning that the new proposal loan amount will only go up) and the fact that the new refinancing proposals should have a collateral amount that is either equal or smaller than the current loan, users will eventually reach the 100% collateralization ratio, and no more refinances would be possible, leading to a DoS of the service per user, and to the auto-refinancing mechanism as if one refinancing reverts, then the whole array of refinances will also revert.

Root Cause

As discussed, whenever a borrower uses the refinance option, the fullfilmentAmount needed to be covered by the new loan, has to include the protocol's fee, meaning that the next refinance will need to have a loanAmount = (debt of borrower) + fee, leading to a constant increase. On the other hand, when doing refinances, the new proposals cannot have a higher collateralAmount due to this check and what is more users will tend to search for proposals that are less collateralized, as they will get any excess collateral back as seen here. This means that eventually the gap between loanAmount and collateralAmount will be reached and no more refinancing will be possible due to the _assertCollateralizationRatioAtLeastOneHundredPercent(...) check. From that point onwards, users will be left with the choice of either to repay or default their borrows. What is more, if the user has opted in for auto-refinancing, and his/her borrow is included in the list, then the whole auto-refinance functionality will revert.

Internal pre-conditions

N/A

External pre-conditions

  1. As there is no info on the refinancing bot, we assume that it includes borrows where the users have opted in for auto refinancing and the PredictDotLoanValidator::validateProposal(...) check passes off-chain.

Attack Path

If we are set to look at this issue as a means of an attack, then the following scenario can be observed, where the auto-refinancing mechanism can be DoSed with dust amounts (there is no loan/collateral amount min/max checks):

NB: the amounts used here are for example reasons to show how the issue unfolds

  1. Alice creates a loan offer with loanAmount = 1000 and collateralAmount = 1050
  2. Alice accepts this offer with another account for a fullfilAmount = 1000.
  3. Alice creates another loan offer with loanAmount = 1020 and the same collateral amount (the increase is needed to cover fees).
  4. Alice refinances the initial loan and pays up the debt.
  5. A new loan is then created with loanAmount = 1040 and is again used to refinance.
  6. From here on no more refinancing can happen as the next refinance will need to have a loan amount > 1050.
  7. However, the collateralAmount cannot be increased as there is a check that reverts when the refinance proposal collateral is larger than the fulfillment one.
  8. Alice opts in for auto-refinancing, and whenever her listing goes in, the whole refinance chain will revert

Impact

  1. As there is no warning regarding having a cap for refinancing, users can end up in situations where they cannot refinance their loans anymore, and will have to either default or repay.
  2. DoS of the auto-refinance mechanism

PoC

The following PoC can be added in the PredictDotLoan.acceptLoanOffer.t.sol test file. I have added the following test helper function:

function _signProposalBorrower2(IPredictDotLoan.Proposal memory proposal)
        internal
        view
        returns (bytes memory signature)
    {
        signature = _signProposal(proposal, borrower2PrivateKey);
    }
 function testDosRefinanceDueToCollateralizationCap() public {
        _updateProtocolFeeRecipientAndBasisPoints(200);

        mockERC20.mint(lender, 1000);
        vm.prank(lender);
        mockERC20.approve(address(predictDotLoan), 1000);

        IPredictDotLoan.Proposal memory loanProposal = _generateLoanOffer(IPredictDotLoan.QuestionType.Binary);
        loanProposal.loanAmount = 1000;
        loanProposal.collateralAmount = 1050;
        loanProposal.signature = _signProposal(loanProposal);

        vm.startPrank(borrower);
        mockCTF.setApprovalForAll(address(predictDotLoan), true);
        vm.stopPrank();

        _mintCTF(borrower);

        vm.prank(borrower);
        predictDotLoan.acceptLoanOffer(loanProposal, 1000);

        IPredictDotLoan.Proposal memory loanProposal2 = _generateLoanOffer(IPredictDotLoan.QuestionType.Binary);
        loanProposal2.loanAmount = 1020;
        loanProposal2.collateralAmount = 1050;
        loanProposal2.from = borrower2;
        loanProposal2.signature = _signProposalBorrower2(loanProposal2);

        mockERC20.mint(borrower2, 1020);
        vm.prank(borrower2);
        mockERC20.approve(address(predictDotLoan), 1020);

        vm.prank(borrower);
        predictDotLoan.refinance(IPredictDotLoan.Refinancing(1, loanProposal2));

        IPredictDotLoan.Proposal memory loanProposal3 = _generateLoanOffer(IPredictDotLoan.QuestionType.Binary);
        loanProposal3.loanAmount = 1041;
        loanProposal3.collateralAmount = 1050;
        loanProposal3.signature = _signProposal(loanProposal3);

        mockERC20.mint(lender, 1041);
        vm.prank(lender);
        mockERC20.approve(address(predictDotLoan), 1041);

        vm.prank(borrower);
        predictDotLoan.refinance(IPredictDotLoan.Refinancing(2, loanProposal3));

        // The next refinance will need to have a loan amount > 1050, if I try with a lower value I get fulfilment too high error
        IPredictDotLoan.Proposal memory loanProposal4 = _generateLoanOffer(IPredictDotLoan.QuestionType.Binary);
        loanProposal4.loanAmount = 1050;
        loanProposal4.collateralAmount = 1050;
        loanProposal4.from = borrower2;
        loanProposal4.signature = _signProposalBorrower2(loanProposal4);

        mockERC20.mint(borrower2, 1050);
        vm.prank(borrower2);
        mockERC20.approve(address(predictDotLoan), 1050);

        vm.prank(borrower);
        vm.expectRevert(IPredictDotLoan.FulfillAmountTooHigh.selector);
        predictDotLoan.refinance(IPredictDotLoan.Refinancing(3, loanProposal4));

        // We need to increase the loan amount, but we also need to increase the collateral ratio due to the 100% collaterailization cap

        IPredictDotLoan.Proposal memory loanProposal5 = _generateLoanOffer(IPredictDotLoan.QuestionType.Binary);
        loanProposal5.loanAmount = 1060;
        loanProposal5.collateralAmount = 1100;
        loanProposal5.from = borrower2;
        loanProposal5.signature = _signProposalBorrower2(loanProposal5);

        mockERC20.mint(borrower2, 1060);
        vm.prank(borrower2);
        mockERC20.approve(address(predictDotLoan), 1060);

        vm.prank(borrower);
        vm.expectRevert(IPredictDotLoan.InsufficientCollateral.selector);
        predictDotLoan.refinance(IPredictDotLoan.Refinancing(3, loanProposal5));

        // If this borrow is included in the auto-refinance list it will revert
        IPredictDotLoan.Refinancing[] memory refinancings = new IPredictDotLoan.Refinancing[](1);
        refinancings[0] = IPredictDotLoan.Refinancing(3, loanProposal5);

        vm.prank(borrower);
        predictDotLoan.toggleAutoRefinancingEnabled();

        vm.prank(bot);
        vm.expectRevert(IPredictDotLoan.InsufficientCollateral.selector);
        predictDotLoan.refinance(refinancings);
    }

Mitigation

This issue is not that straightforward to fix, as the fees are required to avoid any refinancing abuse. Proper warnings should be added to the docs to inform users about this. To minimize the auto-refinance impact, the function could be made to not revert on singular failures, but to maybe emit an event and inform the off-chain mechanism, so that if one fails, the other succeeds.