Boost _ Folks Finance 34124 - [Smart Contract - Low] Smart contract cannot be accessed during the normal liquidation process that involves fully acquiring the borrowers balance

Submitted on Mon Aug 05 2024 17:37:39 GMT-0400 (Atlantic Standard Time) by @ICP for Boost | Folks Finance

Report ID: #34124

Report type: Smart Contract

Report severity: Low

Target: https://testnet.snowtrace.io/address/0xf8E94c5Da5f5F23b39399F6679b2eAb29FE3071e

Impacts:

• Smart contract unable to operate due to lack of token funds

Description

Brief/Intro

Liquidator will ended with underflow exception while normal liquidation process acquiring the borrow balance of the violator

Vulnerability Details

Liquidation is normal process in market when loan went to underCollateralized normally liquidator acquire the assets by calling executeLiquidate() function in LoanManagerLogic from hub. But inside this this function which calls the updateLiquidationBorrows() to transfer the funds from the Violator to liquidator here it calls the transferBorrowFromViolator() function in order to repay and decrease the balance of the violator . Below code snippet we can see :-

    /// @dev Calc the borrow balance and amount to repay and decrease them from violator borrow
    /// @param loan The user loan to transfer the borrow from
    /// @param poolId The pool ID of the borrow
    /// @param repayBorrowAmount The amount to repay
    /// @return repaidBorrowAmount The borrow amount repaid
    /// @return repaidBorrowBalance The borrow balance repaid
    function transferBorrowFromViolator(
        LoanManagerState.UserLoan storage loan,
        uint8 poolId,
        uint256 repayBorrowAmount
    ) external returns (uint256 repaidBorrowAmount, uint256 repaidBorrowBalance, uint256 loanStableRate) {
        LoanManagerState.UserLoanBorrow storage loanBorrow = loan.borrows[poolId];

        // violator loanBorrow has beed updated in prepareLiquidation

        repaidBorrowBalance = repayBorrowAmount;
        repaidBorrowAmount = Math.min(repaidBorrowBalance, loanBorrow.amount);// @audit check here
        loanStableRate = loanBorrow.stableInterestRate;

        loanBorrow.amount -= repaidBorrowAmount;
        loanBorrow.balance -= repaidBorrowBalance; // @audit check here

        if (loanBorrow.balance == 0) clearBorrow(loan, poolId);
    }

In above we see that balance will be decreased by the liquidator parameter repayAmount. [https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/main/contracts/hub/Hub.sol#L118] Which will directly subtract with violator borrow balance after that if the borrow balance is zero then it will clear the borrow here is main issue without validation the user input it will directly subtracts with loanBorrow.

For Scenario We look below numbers :-

 Violator Borrow Balance Before Liquidation 1000000000n

Violator Collateral Balance Before Liquidation 1000000000000000000n

Liquidator Borrow Balance Before Liquidation 0n

Liquidator Collateral Balance Before Liquidation 1000000000n

Repay Amount 965000000n

In above we can see the balances of violator and liquidator before liquidation which is fetch from the user defined functions.

Violator have borrow 1000 USDC and underCollateralized Liquidator tries to acquire by paying 965 USDC but it will halt the process by panic code 0x11 underFlow or overflow Vm exception. Liquidator by seeing the violator borrow balance and try to acquire by lesser amount than the borrow balance it will landed in halted in hub chain.

Impact Details

1 . Temporary blocks the liquidation process and liquidator funds.

2 . Liquidator doesn't allow to acquire the default loans ended with exception which will cause halt of normal process in VM because it not handle with try catch.

3 . Smart contracts which is associated with Liquidation Logic will not be able to access due to the error while acquiring the borrow balance.

The above impact assessed with protocol, if any query please ping me.

code Snippet

https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/main/contracts/hub/logic/LoanManagerLogic.sol#L477C1-L483C10 https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/main/contracts/hub/logic/UserLoanLogic.sol#L127C1-L128C51

Recommendation

1 . Add the check if repayBorrowAmount which is liquidator input is greater than the violator balance cause the panic error if so, then no need to subtract

if( loanBorrow.balance > repaidBorrowBalance) {
loanBorrow.balance -= repaidBorrowBalance;
}

Then we can clear the balance.

2 . Add mechanism that how much can liquidator can repay to acquire full borrow balance to prevent the panic code error.

Proof of concept

Proof of Concept

import { expect } from "chai";
import { ethers } from "hardhat";
import { PANIC_CODES } from "@nomicfoundation/hardhat-chai-matchers/panic";
import { loadFixture, reset, time } from "@nomicfoundation/hardhat-toolbox/network-helpers";
import {
  LiquidationLogic__factory,
  LoanManagerLogic__factory,
  LoanManager__factory,
  LoanPoolLogic__factory,
  MockHubPool__factory,
  MockOracleManager__factory,
  RewardLogic__factory,
  UserLoanLogic__factory,
  LoanManagerStateExposed__factory
} from "../../typechain-types";
import { BYTES32_LENGTH, convertStringToBytes, getAccountIdBytes, getEmptyBytes, getRandomBytes } from "../utils/bytes";
import { SECONDS_IN_DAY, SECONDS_IN_HOUR, getLatestBlockTimestamp, getRandomInt } from "../utils/time";
import { UserLoanBorrow, UserLoanCollateral } from "./libraries/assets/loanData";
import { getNodeOutputData } from "./libraries/assets/oracleData";
import {
  calcAverageStableRate,
  calcBorrowBalance,
  calcBorrowInterestIndex,
  calcReserveCol,
  calcStableInterestRate,
  convToCollateralFAmount,
  convToRepayBorrowAmount,
  convToSeizedCollateralAmount,
  toFAmount,
  toUnderlingAmount,
} from "./utils/formulae";

describe("LoanManager (unit tests)", () => {
  const DEFAULT_ADMIN_ROLE = getEmptyBytes(BYTES32_LENGTH);
  const LISTING_ROLE = ethers.keccak256(convertStringToBytes("LISTING"));
  const ORACLE_ROLE = ethers.keccak256(convertStringToBytes("ORACLE"));
  const HUB_ROLE = ethers.keccak256(convertStringToBytes("HUB"));

  async function deployLoanManagerFixture() {
    const [admin, hub, user, ...unusedUsers] = await ethers.getSigners();

    // libraries
    const userLoanLogic = await new UserLoanLogic__factory(user).deploy();
    const userLoanLogicAddress = await userLoanLogic.getAddress();
    const loanPoolLogic = await new LoanPoolLogic__factory(user).deploy();
    const loanPoolLogicAddress = await loanPoolLogic.getAddress();
    const liquidationLogic = await new LiquidationLogic__factory(
      {
        ["contracts/hub/logic/UserLoanLogic.sol:UserLoanLogic"]: userLoanLogicAddress,
      },
      user
    ).deploy();
    const liquidationLogicAddress = await liquidationLogic.getAddress();
    const loanManagerLogic = await new LoanManagerLogic__factory(
      {
        ["contracts/hub/logic/UserLoanLogic.sol:UserLoanLogic"]: userLoanLogicAddress,
        ["contracts/hub/logic/LoanPoolLogic.sol:LoanPoolLogic"]: loanPoolLogicAddress,
        ["contracts/hub/logic/LiquidationLogic.sol:LiquidationLogic"]: liquidationLogicAddress,
      },
      user
    ).deploy();
    const loanManagerLogicAddress = await loanManagerLogic.getAddress();
    const rewardLogic = await new RewardLogic__factory(user).deploy();