cats - Front-Running rollLoan With newTermsForRoll Forces High Interest Rate on Loan

[sherlock-admin]

cats

high

Front-Running rollLoan With newTermsForRoll Forces High Interest Rate on Loan

Summary

Vulnerability related to front-running a borrower's transaction and forcing him to take on a high interest rate.

Vulnerability Detail

The function provideNewTermsForRoll() in Cooler.sol gives us the opportunity to propose new terms for an existing loanID:

function provideNewTermsForRoll(
        uint256 loanID_,
        uint256 interest_,
        uint256 loanToCollateral_,
        uint256 duration_
    ) external {
        Loan storage loan = loans[loanID_];

        if (msg.sender != loan.lender) revert OnlyApproved();

        loan.request =
            Request(
                loan.amount,
                interest_,
                loanToCollateral_,
                duration_,
                true
            );
    }

Where loanID specifices the index of the existing loan. Here we can propose a new interest rate, loanToCollateral & duration.

The borrower can review the new terms and call rollLoan() to accept the roll as long as the block.timestamp > loan.expiry and loan.request.active == true (which is set to true when proposing new roll terms).

function rollLoan(uint256 loanID_) external {
        Loan memory loan = loans[loanID_];

        if (block.timestamp > loan.expiry) revert Default();
        if (!loan.request.active) revert NotRollable();

        // Check whether rolling the loan requires pledging more collateral or not (if there was a previous repayment).
        uint256 newCollateral = newCollateralFor(loanID_);
        uint256 newDebt = interestFor(loan.amount, loan.request.interest, loan.request.duration);

        // Update memory accordingly.
        loan.amount += newDebt;
        loan.collateral += newCollateral;
        loan.expiry += loan.request.duration;
        loan.request.active = false;

        // Save updated loan info in storage.
        loans[loanID_] = loan;

        if (newCollateral > 0) {
            collateral().safeTransferFrom(msg.sender, address(this), newCollateral);
        }

        // If necessary, trigger lender callback.
        if (loan.callback) CoolerCallback(loan.lender).onRoll(loanID_, newDebt, newCollateral);
    }

The issue stems from the fact that the lender (attacker) can front-run ther borrower's call to rollLoan() and change the conditions for the loan to a much higher interest rate than first proposed, forcing the borrower into a bad loan. Since we are not proposing a new loanID but rather overwriting data on the existing one, the borrower's call goes through and he accepts the malicious attack conditions. Since there is no cap on interest rate this is especially problematic.

Order of attack:

1. A cooler is generated for e.g. a USDC/USDT debt/collateral pair.
2. Borrower requests a loan and malicious lender clears the request.
3. Some time passes and malicious lender proposes new, favorable conditions with low interest rate and extended loan duration.
4. Borrower is happy to agree on the new conditions and calls rollLoan().
5. Malicious lender monitors the mempool and front-runs the borrower, changing the loan conditions to a much higher interest rate for the same loanID.
6. Borrower's transaction goes through and he accepts the new malicious loan conditions.
7. Borrower is stuck with bad loan and might be forced to default on it.

Impact

Borrower is forced to accept bad conditions for his loan and cannot do anything about it, might also lead to the loan defaulting.

Code Snippet

provideNewTermsForRoll()
rollLoan()

Tool used

VSCode, Manual Review

Recommendation

One way to solve this would be to implement a sort of time-lock mechanism that does not allow the lender to change the loan conditions for a set period of time after proposing new ones once. Although it would still be technically possible if the borrower is slow to accept the new terms, it makes it much less likely.

Duplicate of #243