code-423n4 / 2024-06-size-findings

3 stars 1 forks source link

`LiquidateWithReplacement` Fails to Reduce Liquidator Costs and have no benefit over standard `Liquidation` call #41

Closed c4-bot-10 closed 4 months ago

c4-bot-10 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/LiquidateWithReplacement.sol#L120-L163

Vulnerability details

LiquidateWithReplacement() is intended to minimize upfront costs for liquidators by allowing them to fund only a new borrower instead of covering the full debt amount. This should reduce financial exposure compared to standard liquidation. However, the function fails to provide any cost benefits, as liquidators do not receive cash refunds, making it no more advantageous than the standard Liquidate() method.

Furthermore, with only the Keeper Role (same team with the Fee recipient) able to call this function, it results in a financial transfer from the Keeper/Liquidator to the Fee recipient without tangible benefits to the liquidator.

Note: Official Docs does not mention any advantage of using LiquidateWithReplacement() over Liquidate(). I would assume under normal circumstances, it should be more beneficial to use LiquidateWithReplacement().

Impact

The intended financial benefit of reduced costs for liquidators does not materialize with LiquidateWithReplacement(), making it equivalent in effect to using Liquidate().

Proof of Concept

Here is what LiquidateWithReplacement() do:

  1. Initial Liquidation Execution The liquidator triggers a normal liquidation function, where they cover the full debt at maturity to obtain collateral and some profit.
    function executeLiquidateWithReplacement(State storage state, LiquidateWithReplacementParams calldata params)
        external
        returns (uint256 issuanceValue, uint256 liquidatorProfitCollateralToken, uint256 liquidatorProfitBorrowToken)
    {
        emit Events.LiquidateWithReplacement(params.debtPositionId, params.borrower, params.minimumCollateralProfit);
//@note liquidation with replacement can use same borrower or selfborrower
        DebtPosition storage debtPosition = state.getDebtPosition(params.debtPositionId);
        DebtPosition memory debtPositionCopy = debtPosition;
        BorrowOffer storage borrowOffer = state.data.users[params.borrower].borrowOffer;
        uint256 tenor = debtPositionCopy.dueDate - block.timestamp;//timeleft
//@audit LM it is not necessary to send full USDC debt for partial liquidation
        liquidatorProfitCollateralToken = state.executeLiquidate(//@normal liquidation
            LiquidateParams({//@USDC full debt send from msg.sender to Size.
                debtPositionId: params.debtPositionId,//some of collateral transferFrom borrowerPosition to msg.sender
                minimumCollateralProfit: params.minimumCollateralProfit//some collateral fee taken from borrower as well
            })//@ debt is repay in full 100%, claim amount in USDC is calculated for lender.

2.Finding New Borrower and Replacing the Loan The function then calculates a new rate for the borrower and attempts to replace the existing loan with a new borrower, ideally lowering the liquidator's cash outlay.

//@ this liquidationReplacement shorthand for liquidation then current lender call BuyCreditmarket on behalf of current lender. Is it the same as normal operation
        uint256 ratePerTenor = borrowOffer.getRatePerTenor(//@ have internal check for tenor out of range
            VariablePoolBorrowRateParams({
                variablePoolBorrowRate: state.oracle.variablePoolBorrowRate,
                variablePoolBorrowRateUpdatedAt: state.oracle.variablePoolBorrowRateUpdatedAt,
                variablePoolBorrowRateStaleRateInterval: state.oracle.variablePoolBorrowRateStaleRateInterval
            }),
            tenor
        );//@issuanceValue: how much USDC to send to new borrower based on new APR and left over tenor time.
        issuanceValue = Math.mulDivDown(debtPositionCopy.futureValue, PERCENT, PERCENT + ratePerTenor);//@issuanceValue = debt maturity *100% / (new rate percent + 100%)
        liquidatorProfitBorrowToken = debtPositionCopy.futureValue - issuanceValue;//@ previous debt got wiped so must use copy
        //replace currentDebt with new borrower?
        debtPosition.borrower = params.borrower;
        debtPosition.futureValue = debtPositionCopy.futureValue;//@this is still same debt
  1. Resetting Lender Repayment and Transferring Funds The liquidator transfers the full debt maturity to the lender, resetting the lender's claim, which effectively shifts all repayment funds from lender to the Size contract.
        debtPosition.liquidityIndexAtRepayment = 0;//@ reset lender claim. prevent lender from claiming debt repayment
  1. Issuing Funds to the New Borrower Part of the cash from this transaction is then issued to the new borrower as a new loan.
        state.data.debtToken.mint(params.borrower, debtPosition.futureValue);//@new borrower take in full debt of previous borrower. collateral still must meet 150% CR later
        state.data.borrowAToken.transferFrom(address(this), params.borrower, issuanceValue);//cash from new debt
  1. Misallocation of Remaining Funds The remainder of the cash, which could serve as a discount to the liquidator, is instead transferred to the fee recipient, negating any supposed cost-saving benefits of this function.
state.data.borrowAToken.transferFrom(address(this), state.feeConfig.feeRecipient, liquidatorProfitBorrowToken);

Tools Used

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/LiquidateWithReplacement.sol#L120-L163

Recommended Mitigation Steps

Some of liquidatorProfitBorrowToken should transfered back to liquidator/Keeper as a discount for calling LiquidateWithReplacement().

Assessed type

Other

hansfriese commented 4 months ago

Intended design

c4-judge commented 4 months ago

hansfriese marked the issue as unsatisfactory: Invalid