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

5 stars 4 forks source link

web3tycoon - Denial of Service Attack due to `reverting` if `borrower` has turned off `AutoRefinancing` #307

Open sherlock-admin3 opened 1 month ago

sherlock-admin3 commented 1 month ago

web3tycoon

High

Denial of Service Attack due to reverting if borrower has turned off AutoRefinancing

Summary

When AutoRefinace is called with the array of proposals to be autoRefinanced. an attacker can cause all of the loans to go in Auction to seize the collateral. by create a loan adding themselves in the user interface to be under The service. in target of other Loans to be refinance. and once the bot calls the autorefinance. the attackers bot will submit a transaction to disable autorefinance. and when the check if autorefinancedIsEnabled hits the whole transaction will revert, leading to all loans under the service to revert.This will be a waste of gas, and all debts may enter default leading to loss of collateral or paying high amount

Root Cause

In AutoRefinance all loans under the service provided in the user interface may revert if a single loandId ... disables the autorefinance.

if (autoRefinancingEnabled[borrower] == 0) {
                revert BorrowerDidNotEnableAutoRefinancing(borrower);
            }

This will cause revert of all loans under the process

Internal pre-conditions

  1. instead of reverting a whole service for users under autorefinance skip the loanid of the borrower who turns off the autoRefinance

External pre-conditions

No response

Attack Path

  1. The Borrowers join the autoRefinance in the user Interface.
  2. The attacker sees it will be profitable to seize or increase debt by auctioning.
  3. The attacker creates a proposal matches it to a loanOffer
  4. Once he becomes borrower he joins the autorefinance service.
  5. When the service creates a transaction to refinance
  6. The attackers bot frontruns the transaction by turning off refinance https://github.com/sherlock-audit/2024-09-predict-fun/blob/main/predict-dot-loan/contracts/PredictDotLoan.sol#L693C1-L697C6
  7. The transaction is reverted.
  8. This can continue until it is upgraded

Impact

Denial of service through frontrunning and also waste of gas. since the check is after

  for (uint256 i; i < refinancings.length; ++i) {
            Refinancing calldata refinancing = refinancings[i];
            (uint256 id, Loan memory loan, uint256 protocolFee) = _refinance(refinancing);
            //@audit to save on gas add this check before the process of refinancing
            address borrower = loan.borrower;
            if (autoRefinancingEnabled[borrower] == 0) {
                revert BorrowerDidNotEnableAutoRefinancing(borrower);
            }

PoC

No response

Mitigation

Instead of reverting the whole transaction Skip the LoanID that has decided to turnoff the autoRefinance. The ui will not check the frontrunning before hand.

 for (uint256 i; i < refinancings.length; ++i) {
            Refinancing calldata refinancing = refinancings[i];
            (uint256 id, Loan memory loan, uint256 protocolFee) = _refinance(refinancing);

            // Doing this check after the refinancing, but in realitiy
            // it does not matter because the transaction simulation would've
            // failed before it is submitted on-chain.
            //@audit to save on gas add this check before the process of refinancing
            address borrower = loan.borrower;
            if (autoRefinancingEnabled[borrower] == 0) {
 -               revert BorrowerDidNotEnableAutoRefinancing(borrower);
 +             continue;
            }