ni8mare - rollLoan function can be called by anyone.

[sherlock-admin2]

ni8mare

high

rollLoan function can be called by anyone.

Summary

rollLoan can be called by any malicious lender, changing the terms of the loans for the borrower without them wanting to.

Vulnerability Detail

As soon as a loan request is cleared by a lender, the same lender could immediately call the provideNewTermsForRoll and input a very high values for the interest_ and loanToCollateral_ parameters. The call executes since it is called by the same lender who cleared the loan and hence the check (if (msg.sender != loan.lender) revert OnlyApproved();) passes.

As there is no check on who calls the rollLoan function, the same lender can call this function before loan expiry. Also, since loanToCollateral_ was set to a very high value in the provideNewTermsForRoll function, the collateral required to be transferred can be a very small value and even zero depending on the value of loanToCollateral_. The required collateral is defined as:

    function collateralFor(uint256 amount_, uint256 loanToCollateral_) public view returns (uint256) {
        return (amount_ * (10 ** collateral().decimals())) / loanToCollateral_;
    }

So, if loanToCollateral_ is a big value, the collateral returned is a small value. Hence, the new collateral value can even be 0.

    function newCollateralFor(uint256 loanID_) public view returns (uint256) {
        Loan memory loan = loans[loanID_];
        // Accounts for all outstanding debt (borrowed amount + interest).
        uint256 neededCollateral = collateralFor(
            loan.amount,
            loan.request.loanToCollateral
        );

        return
            neededCollateral > loan.collateral ?
            neededCollateral - loan.collateral :
            0; //Note - zero can be returned. Meaning malicious lender need not transfer any collateral
    }

Impact

The debt that a borrower has to pay could be increased to an extremely high value by a malicious lender such that a user could even fail to repay it and in the process, the lender would steal the collateral after the loan's expiry.

Code Snippet

https://github.com/sherlock-audit/2023-08-cooler/blob/main/Cooler/src/Cooler.sol#L192

Tool used

Manual Review

Recommendation

Add a check to see that the rollLoan function is only called by the owner of the escrow, like rescindRequest function.

 if (msg.sender != owner()) revert OnlyApproved();

Duplicate of #26