Ethereum Credit Guild - Inference's results

Lines of code

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

Vulnerability details

Impact

The protocol may incur bad debt than necessary, since it may not always be feasible to achieve the optimal arrangement for ECG, which involves repaying as much of the entire called loan as possible, due to a failing contract in certain conditions.

Proof of Concept

Called loans in the LendingTerm contract undergo auctioning in the AuctionHouse contract. The outstanding debt is calculated prior to submission to the AuctionHouse.

Upon completion of a bid in an auction, the auction concludes. If the bid occurs within the auction period where the requested "callDept" remains constant but the offered collateral increases, the bid may not be settled if there have been additional badDept events since the start of the auction. This is due to the recalculation of the principal in the onBid function, leading to the contract failure in line 766, as the collateralToBorrower would not be zero in this scenario.

The loanDebt calculated in the getLoanDebt function when starting an auction is determined by:

        uint256 borrowAmount = loan.borrowAmount;
        uint256 interest = (borrowAmount *
            params.interestRate *
            (block.timestamp - borrowTime)) /
            YEAR /
            1e18;
        uint256 loanDebt = borrowAmount + interest;
        uint256 _openingFee = params.openingFee;
        if (_openingFee != 0) {
            loanDebt += (borrowAmount * _openingFee) / 1e18;
        }
        uint256 creditMultiplier = ProfitManager(refs.profitManager)
            .creditMultiplier();
        loanDebt = (loanDebt * loan.borrowCreditMultiplier) / creditMultiplier;

The principal in onBid is calculated as:

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

Therefore, the case 'creditFromBidder < principal' in line 761 occurs when the "creditMultiplier," responsible for accounting for bad debt, sets off the interests and openingFee. This happens when the creditMultiplier decreases by the formula (InterestFee * creditMultiplier_at_auction_start) / (borrowAmount + InterestFee), where InterestFee is calculated as (borrowAmount * _openFee + interest).

Tools Used

None. Manual review

Recommended Mitigation Steps

Consider reflecting such a case where 'creditFromBidder < principal' by setting collateralToBorrower value to zero instead failing, if the value is not zero.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/SurplusGuildMinter.sol#L229

Vulnerability details

Impact

The user loses their staked CREDITs when they start staking in a LendingTerm that has previously incurred a loss, as the tokens are written off within the SurPlusGuildMinter contract.

Proof of Concept

The getRewards function in the SurplusGuildMinter contract initializes an empty UserStake object within the definition of the function's return values.

Thus, in L229 within the getRewards function, the code literally checks the following:

lastGaugeLoss = GuildToken(guild).lastGaugeLoss(term);
if (lastGaugeLoss > 0) {
            slashed = true;
        }

Thus, 'slashed' is set to 'true' as soon as the corresponding LendingTerm has already incurred a loss, regardless of when the user started staking their CREDITs.

Therefore, when a user unstakes, all of their CREDITS and GUILD tokens are written off within the getRewards function, even if there was no loss since the user started staking.

Tools Used

Manual review.

Recommended Mitigation Steps

Load the user's userStake before check the "lastGaugeLoss" value.

--- SurplusGuildMinter.sol      2023-12-20 11:11:08.380882238 +0100
+++ SurplusGuildMinter_fixed.sol        2023-12-20 11:16:41.511329264 +0100
@@ -226,12 +226,12 @@
     {
         bool updateState;
         lastGaugeLoss = GuildToken(guild).lastGaugeLoss(term);
+        userStake = _stakes[user][term]; 
         if (lastGaugeLoss > uint256(userStake.lastGaugeLoss)) {
             slashed = true;
         }
 
         // if the user is not staking, do nothing
-        userStake = _stakes[user][term];
         if (userStake.stakeTime == 0)
             return (lastGaugeLoss, userStake, slashed);
 

Assessed type

Invalid Validation