code-423n4 / 2023-12-ethereumcreditguild-findings

9 stars 5 forks source link

Upgraded Q -> 2 from #628 [1706653623677] #1283

Closed c4-judge closed 5 months ago

c4-judge commented 5 months ago

Judge has assessed an item in Issue #628 as 2 risk. The relevant finding follows:

L-02 - The LendingTerm::forgive() function does not validate that the loan is on call

The LendingTerm::forgive() function helps the protocol accept loan losses and update the system accordingly. On the other hand, the LendingTerm::_call() function helps to put the loan up for auction.

The problem is that the LendingTerm::forgive() function does not validate that the loan is on call or in auction, causing the system to update incorrectly.

Consider the following scenario:

  1. For some reason, there is a proposal to call LendingTerm::forgive() on loanId 10.
  2. The loanId 10 enters the auction and the variable loans[loanId].callTime is set to block.timestamp.
  3. The proposal from step 1 is executed before the auction ends, setting loans[loanId].closeTime to the value of block.timestamp.
  4. The auction continues and is reversed by the following validation, making the auction unable to be completed since the code will be reversed:
    File: LendingTerm.sol
    738: require(loans[loanId].closeTime == 0, "LendingTerm: loan closed");
  5. Since the auction will always be reversed in LendingTerm::onBid() the nAuctionsInProgress variable will not be updated, not allowing the variable to be subtracted and an auction house cannot be updated using the LendingTerm::setAuctionHouse() function.

Recommended Mitigation Steps

The LendingTerm::forgive() function must validate that the loan is not up for auction:

    function forgive(bytes32 loanId) external onlyCoreRole(CoreRoles.GOVERNOR) {
        Loan storage loan = loans[loanId];

        // check that the loan exists
        require(loan.borrowTime != 0, "LendingTerm: loan not found");

        // check that the loan is not already closed
        require(loan.closeTime == 0, "LendingTerm: loan closed");

++      // Check the loan is not in auction
++      require(loan.callTime == 0, "LendingTerm: loan on cal");

        // close the loan
        loans[loanId].closeTime = block.timestamp;
        issuance -= loan.borrowAmount;

        // mark loan as a total loss
        uint256 creditMultiplier = ProfitManager(refs.profitManager)
            .creditMultiplier();
        uint256 borrowAmount = loans[loanId].borrowAmount;
        uint256 principal = (borrowAmount *
            loans[loanId].borrowCreditMultiplier) / creditMultiplier;
        int256 pnl = -int256(principal);
        ProfitManager(refs.profitManager).notifyPnL(address(this), pnl);

        // set hardcap to 0 to prevent new borrows
        params.hardCap = 0;

        // emit event
        emit LoanClose(block.timestamp, loanId, LoanCloseType.Forgive, 0);
    }
c4-judge commented 5 months ago

Trumpero marked the issue as duplicate of #1014

c4-judge commented 5 months ago

Trumpero marked the issue as satisfactory

c4-judge commented 5 months ago

This auto-generated issue was withdrawn by Trumpero