The creation of bad debt (mark-down of Credit) can force other loans in auction to also create bad debt

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L751-L768
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L664-L668

Vulnerability details

Bug Description

The creation of bad debt results in the mark down of Credit, i.e. Credit experiences inflation. This is done so that the protocol can adjust to the bad debt that was produced. It follows that when Credit is marked down all active loans can be considered marked up, meaning that the borrowers will be expected to repay with more Credit, since Credit is now worth less. We can observe how this is done by examining the LendingTerm::getLoanDebt function:

LendingTerm::getLoanDebt#L216-L230

216:        // compute interest owed
217:        uint256 borrowAmount = loan.borrowAmount;
218:        uint256 interest = (borrowAmount *
219:            params.interestRate *
220:            (block.timestamp - borrowTime)) /
221:            YEAR /
222:            1e18;
223:        uint256 loanDebt = borrowAmount + interest;
224:        uint256 _openingFee = params.openingFee;
225:        if (_openingFee != 0) {
226:            loanDebt += (borrowAmount * _openingFee) / 1e18;
227:        }
228:        uint256 creditMultiplier = ProfitManager(refs.profitManager)
229:            .creditMultiplier();
230:        loanDebt = (loanDebt * loan.borrowCreditMultiplier) / creditMultiplier;

As we can see above, the debt of a borrower (principle + interests) is adjusted by the creditMultiplier. This creditMultiplier starts at 1e18 and gets marked down when bad debt is produced in the system. The loan.borrowCreditMultiplier is the creditMultiplier value at the time that the user took out the loan. Therefore, this function calculates the adjusted debt of a borrower with respect to the amount of bad debt that was produced (inflation of Credit) since the borrower took out the loan. This function is called every time a borrower makes a payment for their loan, ensuring that the appropriate debt is always considered. However, this function is only called once during the entire auction process:

LendingTerm::_call#L664-L668

664:        // update loan in state
665:        uint256 loanDebt = getLoanDebt(loanId);
666:        loans[loanId].callTime = block.timestamp;
667:        loans[loanId].callDebt = loanDebt;
668:        loans[loanId].caller = caller;

The above code is taken from the _call function, which is the starting point for an auction. When a loan misses a required partialPayment or the term is offboarded, this function can be permissionlessly called. As we can see, the debt of the loan is read from the getLoanDebt function and then stored in the loan struct in the callDebt field. This means that the callDebt represents a snapshot of the debt at block.timestamp. Therefore, the callDebt is the loan debt with respect to the creditMultiplier valued at the time that the loan was called. What if the creditMultiplier gets updated after the auction process begins? This would result in the callDebt of the loan being less than what the actual debt of the loan should be (Credit is worth less, but the collateral is worth the same). We can understand the true magnitude of this discrepancy by observing the LendingTerm::onBid function:

LendingTerm::onBid#L750-L768

750:        // compute pnl
751:        uint256 creditMultiplier = ProfitManager(refs.profitManager)
752:            .creditMultiplier();
753:        uint256 borrowAmount = loans[loanId].borrowAmount;
754:        uint256 principal = (borrowAmount *
755:            loans[loanId].borrowCreditMultiplier) / creditMultiplier;
756:        int256 pnl;
757:        uint256 interest;
758:        if (creditFromBidder >= principal) {
759:            interest = creditFromBidder - principal;
760:            pnl = int256(interest);
761:        } else {
762:            pnl = int256(creditFromBidder) - int256(principal);
763:            principal = creditFromBidder;
764:            require(
765:                collateralToBorrower == 0,
766:                "LendingTerm: invalid collateral movement"
767:            );
768:        }

As we can see above, the principle of the loan is calcualted with respect to the current creditMultiplier. The creditFromBidder is the callDebt when the auction is in its first phase:

AuctionHouse::getBidDetail#L133-L136

133:        // first phase of the auction, where more and more collateral is offered
134:        if (block.timestamp < _startTime + midPoint) {
135:            // ask for the full debt
136:            creditAsked = auctions[loanId].callDebt;

This is where the issue lies. Remember, the callDebt represents a snapshot of the loan debt at the time which the loan was called. The callDebt does not consider a potential updated creditMultiplier. Therefore, if a mark down is generated that results in principle > creditFromBidder, then execution of the onBid function would continue on line 762. This will result in a negative pnl being calculated, which ultimately means that this gauge will experience a loss. However, if the collateralToBorrower is not 0, the function will revert. Therefore, when the principle is greater than the callDebt, due to the mark down of Credit, the auction can only receive a bid if the collateralToBorrower is 0. Let us observe the AuctionHouse::bid and AuctionHouse::getBidDetail functions in order to understand what scenario would result in collateralToBorrower == 0:

AuctionHouse::bid#L169-L186

169:        (uint256 collateralReceived, uint256 creditAsked) = getBidDetail(
170:            loanId
171:        );
172:        require(creditAsked != 0, "AuctionHouse: cannot bid 0");
...
180:        LendingTerm(_lendingTerm).onBid(
181:            loanId,
182:            msg.sender,
183:            auctions[loanId].collateralAmount - collateralReceived, // collateralToBorrower
184:            collateralReceived, // collateralToBidder
185:            creditAsked // creditFromBidder
186:        );

As seen above, the onBidDetail function is invoked to retrieve the necessary collateralReceived and creditAsked values. The onBid function is then invoked and the collateralToBorrower is equal to the collateralAmount - collateralReceived. The collateralAmount is the full collateral of the loan. Therefore, if the collateralReceived == collateralAmount, we will have satisfied the following condition: collateralToBorrower == 0. This is exactly what occurs during the second phase of an auction:

AuctionHouse::getBidDetail#L143-L146

143:        // second phase of the auction, where less and less CREDIT is asked
144:        else if (block.timestamp < _startTime + auctionDuration) {
145:            // receive the full collateral
146:            collateralReceived = auctions[loanId].collateralAmount;

Therefore, given the situation where a loan is placed in auction and then a large enough mark down of Credit occurs, such that principle > creditFromBidder, only bids occuring during the second phase of the auction will be allowed. In addition, given that principle > creditFromBidder, bad debt will also be produced in this auction.

Lets briefly discuss what scenarios would result in principle > callDebt. Reminder: The callDebt represents the maximum value that creditFromBidder can be. The callDebt is a snapshot of the full debt of a borrower (principle + interests). Therefore, if the mark down results in a percent decrease of Credit greater than the interest of the borrower's loan, then the adjusted principle will be greater than the callDebt. Consider the following situation:

A term has an interest rate of 4%. The term has multiple loans opened and the term is being off-boarded after half a year. Lets assume no loans have been paid off during this time. Therefore, the interest for all loans is ~2%. Suppose a loan undergoes auction before other loans are called and this loan results in the creation of bad debt (worse case scenario), which results in a mark down > 2%. All other loans that are in auction during this mark down will be forced to create bad debt since the adjusted principle for all loans in auction will be greater than the loans' callDebt.

Impact

The creation of bad debt has the potential to force other loans to create additional bad debt if the following conditions are met:

1. The other loans were in auction during the mark down
2. The mark down is greater than the interest owed for the loans
3. The global surplus buffer is not manually replenished before each additional bad debt creation (funds essentially sacrificed)

This can greatly impact the health of the protocol as the Credit token is used for all core functionalities. A mark down is a mechanism to allow the system to properly adjust to the creation of bad debt, however I would argue that the creation of bad debt should not result in other loans being forced to produce losses which can ultimately produce more bad debt.

This has the ability to affect loans in other terms as well. All loans in auction during the mark down, originating from any term in the market, can potentially be forced to produce a loss/bad debt. The magnitude of this additional mark down of Credit will be greater if the affected loans have relatively low interest accrued and a large borrow amount.

Secondary effects are that no user's will be able to bid during the first phase of the auction. This first phase is meant to be an opportunity for the borrower to properly repay their full debt before the second phase begins, where the borrower can potentially be out-bid by another liquidator and lose the opportunity to receive their collateral.

Proof of Concept

The following test demonstrates the scenario described above in which a mark down of Credit results in other loans in auction being forced to create additional bad debt.

Place the test inside of the test/unit/loan/ directory:

// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.13;

import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";

import {Test} from "@forge-std/Test.sol";
import {Core} from "@src/core/Core.sol";
import {CoreRoles} from "@src/core/CoreRoles.sol";
import {MockERC20} from "@test/mock/MockERC20.sol";
import {SimplePSM} from "@src/loan/SimplePSM.sol";
import {GuildToken} from "@src/tokens/GuildToken.sol";
import {CreditToken} from "@src/tokens/CreditToken.sol";
import {LendingTerm} from "@src/loan/LendingTerm.sol";
import {AuctionHouse} from "@src/loan/AuctionHouse.sol";
import {ProfitManager} from "@src/governance/ProfitManager.sol";
import {RateLimitedMinter} from "@src/rate-limits/RateLimitedMinter.sol";

contract BadDebtCreatesBadDebt is Test {
    address private governor = address(1);
    address private guardian = address(2);
    address staker = address(0x01010101);
    address borrower = address(0x02020202);
    address lender = address(0x03030303);
    Core private core;
    ProfitManager private profitManager;
    CreditToken credit;
    GuildToken guild;
    MockERC20 collateral;
    MockERC20 pegToken;
    SimplePSM private psm;
    RateLimitedMinter rlcm;
    AuctionHouse auctionHouse;
    LendingTerm term;

    // LendingTerm params (same as deployment script)
    uint256 constant _CREDIT_PER_COLLATERAL_TOKEN = 1e18; // 1:1
    uint256 constant _INTEREST_RATE = 0.04e18; // 4% APR
    uint256 constant _MAX_DELAY_BETWEEN_PARTIAL_REPAY = 0; 
    uint256 constant _MIN_PARTIAL_REPAY_PERCENT = 0; 
    uint256 constant _HARDCAP = 2_000_000e18; // 2 million

    uint256 public issuance = 0;

    function setUp() public {
        vm.warp(1679067867);
        vm.roll(16848497);
        core = new Core();

        profitManager = new ProfitManager(address(core));
        collateral = new MockERC20();
        pegToken = new MockERC20(); // 18 decimals for easy calculations (deployment script uses USDC which has 6 decimals)
        credit = new CreditToken(address(core), "name", "symbol");
        guild = new GuildToken(
            address(core),
            address(profitManager)
        );
        rlcm = new RateLimitedMinter(
            address(core) /*_core*/,
            address(credit) /*_token*/,
            CoreRoles.RATE_LIMITED_CREDIT_MINTER /*_role*/,
            0 /*_maxRateLimitPerSecond*/,
            0 /*_rateLimitPerSecond*/,
            uint128(_HARDCAP) /*_bufferCap*/
        );
        auctionHouse = new AuctionHouse(address(core), 650, 1800);
        term = LendingTerm(Clones.clone(address(new LendingTerm())));
        term.initialize(
            address(core),
            LendingTerm.LendingTermReferences({
                profitManager: address(profitManager),
                guildToken: address(guild),
                auctionHouse: address(auctionHouse),
                creditMinter: address(rlcm),
                creditToken: address(credit)
            }),
            LendingTerm.LendingTermParams({
                collateralToken: address(collateral),
                maxDebtPerCollateralToken: _CREDIT_PER_COLLATERAL_TOKEN,
                interestRate: _INTEREST_RATE,
                maxDelayBetweenPartialRepay: _MAX_DELAY_BETWEEN_PARTIAL_REPAY,
                minPartialRepayPercent: _MIN_PARTIAL_REPAY_PERCENT,
                openingFee: 0,
                hardCap: _HARDCAP
            })
        );
        psm = new SimplePSM(
            address(core),
            address(profitManager),
            address(credit),
            address(pegToken)
        );
        profitManager.initializeReferences(address(credit), address(guild), address(psm));

        // roles
        core.grantRole(CoreRoles.GOVERNOR, governor);
        core.grantRole(CoreRoles.GUARDIAN, guardian);
        core.grantRole(CoreRoles.CREDIT_MINTER, address(this));
        core.grantRole(CoreRoles.GUILD_MINTER, address(this));
        core.grantRole(CoreRoles.GAUGE_ADD, address(this));
        core.grantRole(CoreRoles.GAUGE_REMOVE, address(this));
        core.grantRole(CoreRoles.GAUGE_PARAMETERS, address(this));
        core.grantRole(CoreRoles.CREDIT_MINTER, address(rlcm));
        core.grantRole(CoreRoles.RATE_LIMITED_CREDIT_MINTER, address(term));
        core.grantRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(term));
        core.grantRole(CoreRoles.CREDIT_MINTER, address(psm));
        core.grantRole(CoreRoles.CREDIT_REBASE_PARAMETERS, address(psm));
        core.renounceRole(CoreRoles.GOVERNOR, address(this));

        // add gauge 
        guild.setMaxGauges(10);
        guild.addGauge(1, address(term));
    }

    function testBadDebtCreatesBadDebt() public {
        // staker increases term debtCeiling
        guild.mint(staker, 1000e18);
        vm.startPrank(staker);
        guild.incrementGauge(address(term), 1000e18);
        vm.stopPrank();

        assertEq(guild.getGaugeWeight(address(term)), 1000e18);

        // term has 12 active loans all with various borrow sizes (1_000_000 in total loans)
        // 1st loan = 80_000e18
        collateral.mint(borrower, 1_000_000e18);
        uint256[] memory borrowAmounts = new uint256[](11);
        bytes32[] memory loanIds = new bytes32[](11);
        borrowAmounts[0] = 1_000e18;
        borrowAmounts[1] = 5_000e18;
        borrowAmounts[2] = 10_000e18;
        borrowAmounts[3] = 25_000e18;
        borrowAmounts[4] = 100_000e18;
        borrowAmounts[5] = 50_000e18;
        borrowAmounts[6] = 300_000e18;
        borrowAmounts[7] = 18_000e18;
        borrowAmounts[8] = 90_000e18;
        borrowAmounts[9] = 250_000e18;
        borrowAmounts[10] = 71_000e18;

        vm.prank(borrower);
        collateral.approve(address(term), 1_000_000e18);

        // create 1st loan (loan that will create bad debt)
        bytes32 loanId;
        vm.startPrank(borrower);
        loanId = term.borrow(80_000e18, 80_000e18);
        vm.roll(block.number + 1);
        vm.warp(block.timestamp + 13);
        vm.stopPrank();

        // create the rest of the loans (loans that will be forced to create bad debt)
        for (uint256 i; i < borrowAmounts.length; i++) {
            vm.startPrank(borrower);
            loanIds[i] = term.borrow(borrowAmounts[i], borrowAmounts[i]);
            vm.roll(block.number + 1);
            vm.warp(block.timestamp + 13);
            vm.stopPrank();
        }
        
        assertEq(term.issuance(), 1_000_000e18);
        assertEq(credit.balanceOf(borrower), 1_000_000e18);
        assertEq(credit.totalSupply(), 1_000_000e18);

        // lenders supply 1_000_000 pegToken in psm (credit.totalSupply == 2_000_000)
        pegToken.mint(lender, 1_000_000e18);
        vm.startPrank(lender);
        pegToken.approve(address(psm), 1_000_000e18);
        psm.mintAndEnterRebase(1_000_000e18);
        vm.stopPrank();

        assertEq(credit.totalSupply(), 2_000_000e18);

        // half a year later all loans accrued ~2% interest
        vm.warp(block.timestamp + (term.YEAR() / 2));
        
        // term is offboarded 
        guild.removeGauge(address(term));
        assertEq(guild.isGauge(address(term)), false);

        // one loan is called before the others and it creates bad debt (markdown > % interest owed by other loans)
        term.call(loanId);

        // no ones bids and loan creates bad debt (worse case scenario)
        vm.warp(block.timestamp + auctionHouse.auctionDuration());
        (, uint256 creditAsked) = auctionHouse.getBidDetail(loanId); 
        assertEq(creditAsked, 0); // phase 2 ended

        // all loans called via callMany right before bad debt + markdown occurs 
        // to demonstrate that any auctions live while markdown occured would be affected (including auctions in diff terms)
        term.callMany(loanIds);

        // bad debt created, i.e. markdown of 4%
        // note that for demonstration purposes there are no surplus buffers
        auctionHouse.forgive(loanId);

        assertEq(term.issuance(), 1_000_000e18 - 80_000e18);
        assertEq(credit.totalSupply(), 2_000_000e18);
        assertEq(profitManager.creditMultiplier(), 0.96e18); // credit marked down

        // no one can bid during phase 1 of any other loans that were in auction when the markdown occured
        // due to principle > creditFromBidder, therefore collateral to borrower must be 0, i.e. all collateral is offered, i.e. must be phase 2
        for (uint256 i; i < loanIds.length; i++) {
            ( , creditAsked) = auctionHouse.getBidDetail(loanIds[i]);
            // verify we are in phase 1 (creditAsked == callDebt)
            assertEq(auctionHouse.getAuction(loanIds[i]).callDebt, creditAsked);
            // attempt to bid during phase 1
            credit.mint(address(this), creditAsked);
            credit.approve(address(term), creditAsked);
            vm.expectRevert("LendingTerm: invalid collateral movement");
            auctionHouse.bid(loanIds[i]);
        }

        // fast forward to the beginning of phase 2
        vm.warp(block.timestamp + auctionHouse.midPoint());
        vm.roll(block.number + 1);

        // all other loans that are in auction will be forced to only receive bids in phase 2
        // bad debt is gauranteed to be created for all these loans, so user's are incentivized to 
        // bid at the top of phase 2. This would result in the liquidator receiving the collateral at a discount. 
        // The loans with less accrued interest and a bigger principle/borrow amount will result in a bigger loss, i.e. greater markdown

        emit log_named_uint("creditMultiplier before updates", profitManager.creditMultiplier());
        
        uint256 collateralReceived;
        for (uint256 i; i < loanIds.length; i++) {
            (collateralReceived, creditAsked) = auctionHouse.getBidDetail(loanIds[i]);
            // verify we are at the top of phase 2 (collateralReceived == collateralAmount | creditAsked == callDebt)
            assertEq(auctionHouse.getAuction(loanIds[i]).callDebt, creditAsked);
            assertEq(auctionHouse.getAuction(loanIds[i]).collateralAmount, collateralReceived);
            // bid at top of phase two (bidder receives collateral at a discount & protocol incurs more bad debt)
            credit.mint(address(this), creditAsked);
            credit.approve(address(term), creditAsked);
            auctionHouse.bid(loanIds[i]);
            multiplierUpdated();
        }
    }

    function multiplierUpdated() internal {
        // credit multiiplier decreases which each auction
        uint256 multiiplier = profitManager.creditMultiplier();

        emit log_named_uint("creditMultiplier updated", multiiplier);
    }
}

Tools Used

manual

Recommended Mitigation Steps

Credit debt is calculated in most areas of the system with respect to the current multiplier, except for during the auction process. I would suggest calculating the callDebt dynamically with respect to the current creditMultiplier during the auction process instead of having it represent a 'snapshot' of the borrower's debt.

Assessed type

Other

[c4-pre-sort]

0xSorryNotSorry marked the issue as high quality report

[0xSorryNotSorry]

The issue is well demonstrated, properly formatted, contains a coded POC.
Marking as HQ.

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #1069

[c4-judge]

Trumpero marked the issue as satisfactory

[c4-judge]

Trumpero changed the severity to 3 (High Risk)

[0xJCN]

Hi @Trumpero,

I understand that labeling a report as selected for report is subjective, however considering the fact that this report contains a coded POC while #1069 does not, I would like to ask you to consider this report for the selected for report label instead.

Thank you for taking the time to read my comment.

[Trumpero]

@0xJCN I agree this issue should be selected for report instead of #1069 due to its PoC test.

[c4-judge]

Trumpero marked the issue as selected for report

[stalinMacias]

@Trumpero Quick question, all of the reports that were duplicates of #1069 will now also be updated to be a duplicate of this one, right?