Active Boosts

Boost _ Folks Finance 34179 - [Smart Contract - High] Incorrect Updates to pooldepositDatatotalAmount and loancollateralUsed During Repayment with Collateral

Submitted on Tue Aug 06 2024 06:41:08 GMT-0400 (Atlantic Standard Time) by @alix_40 for Boost | Folks Finance

Report ID: #34179

Report type: Smart Contract

Report severity: High

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

Impacts:

  • Theft of unclaimed yield

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

Description

This report is intended to be submitted under my team account "A2Security" but I reached the report submission rate limit on the last day. Please count this report as though it were from "A2Security".

Description

  • The protocol's repayment with collateral mechanism contains a mathematical flaw in its accounting logic, specifically affecting the updates of pool.depositData.totalAmount during repayment operations.

  • In a properly functioning system, depositData.totalAmount should never exceed the sum of the pool's actual balance plus the total borrowed amount. This invariant ensures accurate representation of the pool's financial state and is crucial for various protocol operations.

  • The repayment with collateral process involves users repaying their loans using their deposited collateral instead of external funds.However, the current implementation in LoanManagerLogic.sol incorrectly updates the totalAmount

function updateWithRepayWithCollateral(HubPoolState.PoolData storage pool, uint256 principalPaid, uint256 interestPaid, uint256 loanStableRate)
    external
    returns (DataTypes.RepayWithCollateralPoolParams memory repayWithCollateralPoolParams)
{
    // ... other code ...
    pool.depositData.totalAmount -= principalPaid - interestPaid; // Incorrect update
    // ... rest of the function
}

  • It subtracts principalPaid - interestPaid from totalAmount, which effectively reduces the total amount by the principal and increases it by the interest. This is incorrect because the interest should not be added to totalAmount.

  • The issue arises because when a user repays with collateral, the interest they're paying is already accounted in pool.depositData.totalAmount. This amount was included when the user initially deposited their collateral. By adding the interest again during repayment, we're double-counting this value, leading to an artificial inflation of totalAmount.

This mishandling leads to several issues:

  1. pool.depositData.totalAmount becomes inflated, exceeding the actual balance held in the pool.

  2. The discrepancy between totalAmount and the actual pool balance grows over time, compounding with each repayment using collateral.

  3. The inflated totalAmount affects critical calculations such as utilization ratios, interest rates, and liquidity assessments.

  • As an example lets take the following scenario:

  1. A user deposits 1000 USDC into the pool.

    • loan.collateralUsed = 1000 USDC(in terms of FUSDC)

    • pool.depositData.totalAmount = 1000 USDC

    • loan.borrowUsed = 0

    • actual poolHub balance = 1000 USDC

  2. The user borrows 900 USDC against this deposit. A after some time the user repays the loan with his collateral with interest of 60 USDC, the state after the repayment will be :

    • loan.collateralUsed = 40

    • pool.depositData.totalAmount = 160 (incorrectly updated)

    • loan.borrowUsed = 0

    • actual poolHub balance = 100 USDC

This shows a mismatch where pool.depositData.totalAmount is higher than the actual pool balance, which should not happen.

Impact

The inflated totalDepositAmount in the protocol leads to several significant effects:

  • Utilization Ratio: The utilization ratio calculated in HubPoolLogic.sol is artificially lowered, as it uses totalDeposits in the denominator. This makes it appear that a smaller portion of available funds is being utilized.

  • Interest Rates: Both borrowing and lending interest rates, derived from the utilization ratio, are affected. The lower perceived utilization results in the protocol setting lower interest rates than it should, potentially underpricing risk.

  • Deposit Indexes: Deposit interest index calculations are impacted, leading to an underestimation of depositors' share growth over time. T Due to the undervalued deposits index resulting from the false utilization ratio (lower UT means lower Ftoken value ), a portion of user funds become locked in the protocol, as the calculated value of their Ftoken would be less than the actual value.

  • Caps and Limits: Functions like isDepositCapReached or isBorrowCapReached operate with incorrect values.

These effects compound over time, leading to increasing discrepancies between the protocol's perceived state and its actual financial position. which will result in significant financial inaccuracies .

Tools Used

Manual Review

Recomandation :

  • In the updateWithRepayWithCollateral function in LoanManagerLogic.sol we should only subtract the principal from totalAmount:

function updateWithRepayWithCollateral(HubPoolState.PoolData storage pool, uint256 principalPaid, uint256 interestPaid, uint256 loanStableRate)
    external
    returns (DataTypes.RepayWithCollateralPoolParams memory repayWithCollateralPoolParams)
{
    // ... other code ...
-    uint256 fAmountRepaid = principalPaid - interestPaid
+    pool.depositData.totalAmount -= principalPaid;

    // ... rest of the function
}

Proof of concept

Proof of Concept

  • here a coded poc shows how repaying with collateral will lead to an inflated totalDepositAmount that is more than the actual pool balance + total borrowed amount ,

  • please run test with : forge test --mt test_poc_01 -vvv --via-ir

[PASS] test_poc_01() (gas: 1454096)
Logs:
  amount to repay  4327641473
  actual balance before                :  2715350070
  totalDeposited minus borrowed before :  2714348991
  actual balance after                 :  2872986419
  totalDeposited minus borrowed after  :  3199626813

to run test please first add the following file to test/pocs/base_test.sol

   // SPDX-License-Identifier: UNLICENSED
   pragma solidity ^0.8.19;

   import "@forge-std/Test.sol";
   import "contracts/hub/Hub.sol";
   import "contracts/spoke/SpokeToken.sol";
   import "contracts/spoke/SpokeCommon.sol";
   import "contracts/hub/HubPool.sol";
   import "contracts/hub/LoanManager.sol";
   import "contracts/bridge/libraries/Messages.sol";
   import "contracts/hub/AccountManager.sol";
   import "contracts/bridge/BridgeRouter.sol";
   import "contracts/bridge/HubAdapter.sol";
   import "contracts/oracle/modules/NodeManager.sol";
   import "contracts/hub/OracleManager.sol";
   import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

   contract baseTest is Test {
      using SafeERC20 for address;
      // Hub public hub;

      uint256 mainnetFork;
      bytes32 public constant PREFIX = "HUB_ADAPTER_V1";
      address constant HUB_ADDRESS = 0xaE4C62510F4d930a5C8796dbfB8C4Bc7b9B62140; // Assuming this is the Hub address
      address constant SPOKE_COMMON = 0x6628cE08b54e9C8358bE94f716D93AdDcca45b00;
      address constant SPOKE_USDC = 0x89df7db4af48Ec7A84DE09F755ade9AF1940420b;
      address constant HUBPOOL_USDC = 0x1968237f3a7D256D08BcAb212D7ae28fEda72c34;
      address constant HUBPOOL_AVAX = 0xd90B7614551E799Cdef87463143eCe2efd4054f9;
      address constant SPOKE_AVAX = 0xBFf8b4e5f92eDD0A5f72b4b0E23cCa2Cc476ce2a;
      address constant LOAN_MANAGER = 0x2cAa1315bd676FbecABFC3195000c642f503f1C9;
      address constant ACCOUNT_MANAGER = 0x3324B5BF2b5C85999C6DAf2f77b5a29aB74197cc;
      address constant USDC_TOKEN = 0x5425890298aed601595a70AB815c96711a31Bc65;
      address constant ADAPTER = 0xf472ab58969709De9FfEFaeFFd24F9e90cf8DbF9;
      address constant LISTING_ROLE = 0x16870a6A85cD152229B97d018194d66740f932d6;
      address constant BRIDGE_ROUTER_HUB = 0xa9491a1f4f058832e5742b76eE3f1F1fD7bb6837;
      address constant ORACLE_MANAGER = 0x46c425F4Ec43b25B6222bcc05De051e6D3845165;
      address constant NODE_MANAGER = 0xA758c321DF6Cd949A8E074B22362a4366DB1b725;
      uint16 constant STABELE_LOAN_TYPE_ID = 1;
      uint16 constant VARIABLE_LOAN_TYPE_ID = 2;
      uint16 constant CHAIN_ID = 1; // Assuming Ethereum mainnet

      Hub hub;
      SpokeCommon spokeCommon;
      AccountManager accountManager;
      SpokeToken spokeUsdc;