CreditMultiplier is not applied to creditAsked when a bid for an active auction is placed. This can reduce the creditMultiplier and thus the value of creditToken holders.

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L228-L230
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L751-L755
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L758-L767

Vulnerability details

Impact

• auction can't be won as long as auction is in first phase;
• interest is not paid;
• protocol (and credit holders) are forcing to incur a loss when auction enters second phase (100% collateralToBidder, less and less Credit payed back).
• when auction enters second phase, bidder pays less Credit than it should.

Proof of Concept

In case of a late partial repayment or offboarding lendingTerm loans can be called.

LendingTerm.getLoanDebt is used to calculate the outstanding borrowed amount of a loan. It includes the principal, interest, openFee. A creditMultiplier correction is applied to reflect the updated loan value in case creditToken <-> underlyingToken ratio decreased after the loan was opened.

//getLoanDebt
        uint256 creditMultiplier = ProfitManager(refs.profitManager).
            creditMultiplier();
        loanDebt = (loanDebt * loan.borrowCreditMultiplier) / 
            creditMultiplier;

loans mapping is updated with up-to-date loadDebt amount.

//_call
        // update loan in state
        uint256 loanDebt = getLoanDebt(loanId);
        loans[loanId].callTime = block.timestamp;
        loans[loanId].callDebt = loanDebt;

The auction is started. auctions[loanId] store same up-to-data loanDebt (named callDebt now).

        // save auction in state
        auctions[loanId] = Auction({
            startTime: block.timestamp,
            endTime: 0,
            lendingTerm: msg.sender,
            collateralAmount: loan.collateralAmount,
            callDebt: callDebt
        });

Now auction is active and anyone can bid on it.
AuctionHouse.getBidDetail is used to get the collateralReceived (by bidder) for creditAsked.
As we can see updated callDebt (updated at the moment the auction started) is returned for creditAsked. (we ignore the fact less Credit is asked in second phase of the auction).

        if (block.timestamp < _startTime + midPoint) {
            // ask for the full debt
>>          creditAsked = auctions[loanId].callDebt;

            // compute amount of collateral received
            uint256 elapsed = block.timestamp - _startTime; // [0, midPoint[
            uint256 _collateralAmount = auctions[loanId].collateralAmount; // SLOAD
            collateralReceived = (_collateralAmount * elapsed) / midPoint;
        }
        // second phase of the auction, where less and less CREDIT is asked
        else if (block.timestamp < _startTime + auctionDuration) {
            // receive the full collateral
            collateralReceived = auctions[loanId].collateralAmount;

            // compute amount of CREDIT to ask
            uint256 PHASE_2_DURATION = auctionDuration - midPoint;
            uint256 elapsed = block.timestamp - _startTime - midPoint; // [0, PHASE_2_DURATION[
            uint256 _callDebt = auctions[loanId].callDebt; // SLOAD
>>          creditAsked = _callDebt - (_callDebt * elapsed) / PHASE_2_DURATION;
        }

creditAsked is passed down to onBid callback.

• The principal is calculated by applying the borrowCreditMultiplier / creditMultiplier correction:

        uint256 creditMultiplier = ProfitManager(refs.profitManager)
            .creditMultiplier();
        uint256 borrowAmount = loans[loanId].borrowAmount;
        uint256 principal = (borrowAmount *
            loans[loanId].borrowCreditMultiplier) / creditMultiplier;

• creditFromBidder is the creditAsked == callDebt == loanDebt updated at the moment auction started. No new correction is applied.

Let me explain what I mean by that and why it matters.

The auction can last many blocks, up to 30 minutes based on in the scope deployment script.

    //GIP_0.sol
            AuctionHouse auctionHouse = new AuctionHouse(
                AddressLib.get("CORE"),
                650, // midPoint = 10m50s
                1800 // auctionDuration = 30m
            );

creditMultiplier can have a correction down (due to bad debt accumulation) between (1) the loan was called (auction started) and (2) the auction was bid on.

On the time axis we have 3 creditMultiplier values of interest :

borrowCreditMultiplier >= auctionStartedCreditMultiplier >= bidOnAuctionCreditMultiplier == creditMultiplier

Going back on onBid callback :

-> principal = borrowAmount * borrowCreditMultiplier/ bidOnAuctionCreditMultiplier

-> creditFromBidder = borrowAmountPlusInterests * borrowCreditMultiplier / auctionStartedCreditMultiplier

In the first phase of auction more and more collateral is offered for the same amount of CREDIT to pay.
Let's suppose a bidder bids in this phase, offering creditFromBidder for a x% ( x < 100%) of collateral.
But, in case creditMultiplier decreased between startAuction and bid moment, principal can be bigger than creditFromBidder amount, forcing the code to enter else branch:

        } else {
            pnl = int256(creditFromBidder) - int256(principal);
            principal = creditFromBidder;
            require(
                collateralToBorrower == 0,
                "LendingTerm: invalid collateral movement"
            );

Because collateralToBorrower must be 0 even if auction is in first half (and collateral is split between bidder and borrower), the bid transaction reverts:

• auction can't be won as long as auction is in first phase;
• interest is not paid;
• protocol (and credit holders) are forcing to incur a loss when auction enters second phase (100% collateralToBidder, less and less Credit payed back).
• when auction enters second phase, bidder pays less Credit than it should.

Tools Used

Manual review

Recommended Mitigation Steps`

Consider applying same correction to both principal and creditFromBidder:

• LendingTerm._call : loans[loanId].callDebt => save rawLoanDebt (principal +interest + openFee) (but do not apply creditMultiplier correction)

• AuctionHouse.getBidDetail : getBidDetail => apply creditM correction to rawLoanDebt to calculate creditAsked:
  creditAsked = rawLoanDebt * borrowCreditMultiplier/ bidOnAuctionCreditMultiplier

• when onBid callback is called the creditAsked updated amount is passed and compared with principal which has same correction applied.

Assessed type

Error

[0xSorryNotSorry]

Looks like the same root cause referred here: #1156
I'll leave this as primary since the issue is better pronounced.

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[eswak]

Very clear, thanks for the quality of the report. Confirming

[c4-sponsor]

eswak (sponsor) confirmed

[c4-judge]

Trumpero marked the issue as satisfactory

[c4-judge]

Trumpero marked the issue as selected for report

[c4-judge]

Trumpero marked issue #476 as primary and marked this issue as a duplicate of 476