A Borrower can Use Fluctuations in Collateral Price to Call his Loans and Pay less Credit than the Actual Debt.

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L339-L435 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L634-L675 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L725-L825 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/AuctionHouse.sol#L166-L196

Vulnerability details

Impact

A Borrower can use collateral price fluctuations(e.g. During bear markets) to call his own loans when the price of collateral is less than the debt value, and use this to pay less debt and get his collateral back.

this is possible with a conjunction of two variables, and because anyone can call loans even the borrower and the borrower can bid for his own loans.

Proof of Concept

Let's see the next full scenario:

Any contract(bot)/user can borrow credit from the ECG protocol by depositing collateral and asking to borrow against it, these loans are overcollateralized with collateral tokens, so a contract can deposit collateral to the system and ask for a debt in a lending term with partial repayments enables.

function _borrow(
        address borrower,
        uint256 borrowAmount,
        uint256 collateralAmount
    ) internal returns (bytes32 loanId) {
        require(borrowAmount != 0, "LendingTerm: cannot borrow 0");
        require(collateralAmount != 0, "LendingTerm: cannot stake 0");

        loanId = keccak256(
            abi.encode(borrower, address(this), block.timestamp)
        );

        // check that the loan doesn't already exist
        require(loans[loanId].borrowTime == 0, "LendingTerm: loan exists");

        // check that enough collateral is provided
        uint256 creditMultiplier = ProfitManager(refs.profitManager)
            .creditMultiplier();
        uint256 maxBorrow = (collateralAmount *
            params.maxDebtPerCollateralToken) / creditMultiplier;
        require(
            borrowAmount <= maxBorrow,
            "LendingTerm: not enough collateral"
        );

        // check that enough CREDIT is borrowed
        require(
            borrowAmount >= ProfitManager(refs.profitManager).minBorrow(),
            "LendingTerm: borrow amount too low"
        );

        // check the hardcap
        uint256 _issuance = issuance;
        uint256 _postBorrowIssuance = _issuance + borrowAmount;
        require(
            _postBorrowIssuance <= params.hardCap,
            "LendingTerm: hardcap reached"
        );

        // check the debt ceiling
        uint256 totalBorrowedCredit = ProfitManager(refs.profitManager)
            .totalBorrowedCredit();
        uint256 gaugeWeightTolerance = ProfitManager(refs.profitManager)
            .gaugeWeightTolerance();
        uint256 _debtCeiling = (GuildToken(refs.guildToken)
            .calculateGaugeAllocation(
                address(this),
                totalBorrowedCredit + borrowAmount
            ) * gaugeWeightTolerance) / 1e18;
        if (totalBorrowedCredit == 0) {
            // if the lending term is deprecated, `calculateGaugeAllocation` will return 0, and the borrow
            // should revert because the debt ceiling is reached (no borrows should be allowed anymore).
            // first borrow in the system does not check proportions of issuance, just that the term is not deprecated.
            require(_debtCeiling != 0, "LendingTerm: debt ceiling reached");
        } else {
            require(
                _postBorrowIssuance <= _debtCeiling,
                "LendingTerm: debt ceiling reached"
            );
        }

        // save loan in state
        loans[loanId] = Loan({
            borrower: borrower,
            borrowTime: block.timestamp,
            borrowAmount: borrowAmount,
            borrowCreditMultiplier: creditMultiplier,
            collateralAmount: collateralAmount,
            caller: address(0),
            callTime: 0,
            callDebt: 0,
            closeTime: 0
        });
        issuance = _postBorrowIssuance;
        if (params.maxDelayBetweenPartialRepay != 0) {
            lastPartialRepay[loanId] = block.timestamp;
        }

        // mint debt to the borrower
        RateLimitedMinter(refs.creditMinter).mint(borrower, borrowAmount);

        // pull the collateral from the borrower
        IERC20(params.collateralToken).safeTransferFrom(
            borrower,
            address(this),
            collateralAmount
        );

        // emit event
        emit LoanOpen(
            block.timestamp,
            loanId,
            borrower,
            collateralAmount,
            borrowAmount
        );
    }

Once the bot has the loan, it has to wait for the conjunction of these two variables:

1. Wait for the collateral to lose value and go under the debt credit value (like in a bear market).

2. Wait for the Max Delay Between Partial Repayments to pass.

   So with the conjunction of these two variables, the bot will call his own loan for auction.

Taking into account that the collateral value is less than the credit debt, no MEV bot nor any user will bid on the loan at least during the first part of the Auction where the collateral is offered and the full debt asked, cause they will be incurring losses if they bid, so the user's contract can wait to bid listening to the Mempool for a bid in his loan.

Once the second Part of the auction starts the whole collateral is offered and less and less credit is asked over time, so the user's bot can wait listening to the Mempool for a bid in his loan and letting the debt asked decrease over time, when the user's bot see a bid for his loan in the Mempool, he can front-run it and buy its own loan getting back his whole collateral and paying less debt credit for the loan.

function _call(
        address caller,
        bytes32 loanId,
        address _auctionHouse
    ) internal {
        Loan storage loan = loans[loanId];

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

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

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

        // check that the loan can be called
        require(
            GuildToken(refs.guildToken).isDeprecatedGauge(address(this)) ||
                partialRepayDelayPassed(loanId),
            "LendingTerm: cannot call"
        );

        // check that the loan has been running for at least 1 block
        require(
            borrowTime < block.timestamp,
            "LendingTerm: loan opened in same block"
        );

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

        // auction the loan collateral
        AuctionHouse(_auctionHouse).startAuction(loanId, loanDebt);

        // emit event
        emit LoanCall(block.timestamp, loanId);
    }

 function bid(bytes32 loanId) external {
        // this view function will revert if the auction is not started,
        // or if the auction is already ended.
        (uint256 collateralReceived, uint256 creditAsked) = getBidDetail(
            loanId
        );
        require(creditAsked != 0, "AuctionHouse: cannot bid 0");

        // close the auction in state
        auctions[loanId].endTime = block.timestamp;
        nAuctionsInProgress--;

        // notify LendingTerm of auction result
        address _lendingTerm = auctions[loanId].lendingTerm;
        LendingTerm(_lendingTerm).onBid(
            loanId,
            msg.sender,
            auctions[loanId].collateralAmount - collateralReceived, // collateralToBorrower
            collateralReceived, // collateralToBidder
            creditAsked // creditFromBidder
        );

        // emit event
        emit AuctionEnd(
            block.timestamp,
            loanId,
            LendingTerm(_lendingTerm).collateralToken(),
            collateralReceived, // collateralSold
            creditAsked // debtRecovered
        );
    }

Tools Used

Manual Review & Foundry

Recommended Mitigation Steps

A solution to this problem could be to deny the borrower from calling his own loans, or not allowing the borrower to bid on his own loans(but I think this is allowed as an opportunity for the borrower to recover his loan).

you can also let the borrower only bid in the first part of the auction and not in the second.

These are some ideas that could be analyzed and implemented to prevent this bad behavior.

Assessed type

Other

[0xSorryNotSorry]

This is a known behavior as per the docs and subject to MEV

[c4-pre-sort]

0xSorryNotSorry marked the issue as insufficient quality report

[Trumpero]

Non-issue. Anyone can call a loan, and Guild's holders should call in the first part of auction to prevent loss and slashing. If the collateral loses value and collateral price goes under maxDebtPerCollateralToken, the lending term should be off-boarded asap.

[c4-judge]

Trumpero marked the issue as unsatisfactory: Invalid