Unable to change the auction house if forgive() is called by the governor on an auctioned loan

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L695-L722

Vulnerability details

Impact

If forgive() is called by the governor on a loan currently undergoing an auction, it prevents the establishment of a new auction house through the setAuctionHouse() call. This issue arises because, in AuctionHouse.sol, the state variable nAuctionsInProgress at line 54 is not decremented to zero when forgive() is called in LendingTerm.sol. Consequently, even in the absence of any auctions in progress, nAuctionsInProgress remains equal to 1, leading to a revert when attempting to execute setAuctionHouse() at lines 836-839. The following check will revert: https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L836-L839

Proof of Concept

Paste the following PoC at the end of LendingTerm.t.sol

    function testPocSetAuctionHouse() public {
        // prepare & borrow
        uint256 borrowAmount = 20_000e18;
        uint256 collateralAmount = 15e18;
        collateral.mint(address(this), collateralAmount);
        collateral.approve(address(term), collateralAmount);
        bytes32 loanId = term.borrow(borrowAmount, collateralAmount);

        // offboard term
        vm.warp(block.timestamp + 13);
        vm.roll(block.number + 1);
        guild.removeGauge(address(term));

        // call
        term.call(loanId);

        // forgive loan
        vm.prank(governor);
        term.forgive(loanId);

        // set new auction house
        vm.startPrank(governor);
        vm.expectRevert("LendingTerm: auctions in progress");
        term.setAuctionHouse(address(this));
        vm.stopPrank();
    }

Tools Used

Manual review

Recommended Mitigation Steps

The Governor should not have the ability to forgive a loan during an auction. Check that loan.callTime == 0 in forgive() function (LendingTerm.sol): require(loan.callTime == 0, "LendingTerm: auction in progress" To forgive a loan during an auction, forgive() can be called by anyone from AuctionHouse.sol at the end of the auction.

Assessed type

DoS

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[eswak]

Acknowledging this, it doesn't hurt to fix the issue (as outlined by the warden, we just have to add a check that the loan is not called in forgive()), however the GOVERNOR is trusted to interact properly with the protocol as it is a very powerful role, so I doubt this situation can happen in an operational context.

[c4-sponsor]

eswak (sponsor) acknowledged

[c4-sponsor]

eswak marked the issue as disagree with severity

[c4-judge]

Trumpero changed the severity to 2 (Med Risk)

[c4-judge]

Trumpero marked the issue as satisfactory

[Trumpero]

Downgrading this issue to low severity since it represents an admin/governor mistake.

[c4-judge]

Trumpero changed the severity to QA (Quality Assurance)

[c4-judge]

Trumpero marked the issue as grade-a