Bigsam - After a User withdraws The interest Rate is not updated accordingly leading to the next user using an inflated index during next deposit before the rate is normalized again

[sherlock-admin4]

Bigsam

Medium

After a User withdraws The interest Rate is not updated accordingly leading to the next user using an inflated index during next deposit before the rate is normalized again

Summary

A bug in Zerolend's withdrawal mechanism causes the interest rate to not be updated when funds are transferred to the treasury during a withdrawal. This failure leads to the next user encountering an inflated interest rate when performing subsequent actions like deposit, withdrawal or borrow before the rate is normalized again. The issue arises because the liquidity in the pool drops due to the funds being transferred to the treasury, but the system fails to update the interest rate to reflect this change.

Root Cause

Examples of update rate before transferring everywhere in the protocol to maintain Rate

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/SupplyLogic.sol#L69-L81

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/SupplyLogic.sol#L125-L146

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/BorrowLogic.sol#L88-L99

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/BorrowLogic.sol#L139-L158

The same process can be observed in Aave v 3.

1. https://github.com/aave/aave-v3-core/blob/782f51917056a53a2c228701058a6c3fb233684a/contracts/protocol/libraries/logic/SupplyLogic.sol#L130
2. https://github.com/aave/aave-v3-core/blob/782f51917056a53a2c228701058a6c3fb233684a/contracts/protocol/libraries/logic/SupplyLogic.sol#L65
3. https://github.com/aave/aave-v3-core/blob/782f51917056a53a2c228701058a6c3fb233684a/contracts/protocol/libraries/logic/BorrowLogic.sol#L145-L150
4. https://github.com/aave/aave-v3-core/blob/782f51917056a53a2c228701058a6c3fb233684a/contracts/protocol/libraries/logic/BorrowLogic.sol#L227-L232

Looking at the effect of updating rate

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/ReserveLogic.sol#L134-L182

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/periphery/ir/DefaultReserveInterestRateStrategy.sol#L98-L131

This rates are used to get the new index

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/ReserveLogic.sol#L225-L227

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/ReserveLogic.sol#L235-L237

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

In the current implementation of Zerolend, during a withdrawal, the protocol transfers a portion of the funds to the treasury. However, it does not update the interest rate before this transfer has being done for all transfers, leading to an inflated liquidity rate being used by the next user, particularly for deposits. This is problematic as the next user deposits/withdraws at a rate that is incorrectly high, causing them to receive fewer shares than they should.

In comparison, Aave mints shares to the treasury, which can later withdraw this funds like any other user.

Each withdrawal out of the contract in underlying asset in Aave updates the interest rate, ensuring the rates reflect the true liquidity available in the pool.

Zerolend's approach of transferring funds directly upon every user withdrawal fails to adjust the interest rate properly, resulting in a temporary discrepancy that affects subsequent users.

Code Context:

In the executeMintToTreasury function, the accrued shares for the treasury are transferred, but the interest rates are not updated to account for the change in liquidity.

function executeMintToTreasury(
    DataTypes.ReserveSupplies storage totalSupply,
    mapping(address => DataTypes.ReserveData) storage reservesData,
    address treasury,
    address asset
  ) external {
    DataTypes.ReserveData storage reserve = reservesData[asset];

    uint256 accruedToTreasuryShares = reserve.accruedToTreasuryShares;

    if (accruedToTreasuryShares != 0) {
      reserve.accruedToTreasuryShares = 0;
      uint256 normalizedIncome = reserve.getNormalizedIncome();
      uint256 amountToMint = accruedToTreasuryShares.rayMul(normalizedIncome);

@audit>> no interest rate update before fund removal >>       IERC20(asset).safeTransfer(treasury, amountToMint);

      totalSupply.supplyShares -= accruedToTreasuryShares;

      emit PoolEventsLib.MintedToTreasury(asset, amountToMint);
    }
  }

As can be seen in this snippet, the funds are transferred to the treasury, but the function does not invoke any interest rate update mechanism. The liquidity in the pool decreases, but the next user's deposit will use an inflated rate due to this oversight.

Interest Rate Update Example (Correct Flow):

In other parts of the code, such as during withdrawals, the interest rate is properly updated when liquidity changes:

 function executeWithdraw(
    mapping(address => DataTypes.Res

---------------------------------------
reserve.updateInterestRates(
  totalSupplies,
  cache,
  params.asset,
  IPool(params.pool).getReserveFactor(),
  0,  // No liquidity added
  params.amount,  // Liquidity taken during withdrawal
  params.position,
  params.data.interestRateData
);

The updateInterestRates function correctly calculates the new interest rate based on the changes in liquidity, ensuring the system uses accurate rates for subsequent operations.

Example of Problem:

Consider the following scenario:

• A user withdraws a portion of funds, which triggers the transfer of some assets to the treasury.
• The liquidity in the pool drops, but the interest rate is not updated.
• The next user deposits into the pool using the inflated liquidity rate, resulting in fewer shares being minted for them.

Since the actual liquidity is lower than the interest rate assumes, the user depositing gets fewer shares than expected.

---

Impact

• Incorrect Share Calculation: Users depositing after a treasury withdrawal will receive fewer shares due to an artificially high liquidity rate than the appropriate one , leading to loss of potential value.

PoC

No response

Mitigation

The mitigation involves ensuring that the interest rate is properly updated before transferring funds to the treasury. The rate update should account for the liquidity being transferred out, ensuring the new rates reflect the actual available liquidity in the pool.

Suggested Fix:

In the executeMintToTreasury function, call the updateInterestRates function before transferring the assets to the treasury. This will ensure that the interest rate reflects the updated liquidity in the pool before the funds are moved.

Modified Code Example:

function executeMintToTreasury(
    DataTypes.ReserveSupplies storage totalSupply,
    mapping(address => DataTypes.ReserveData) storage reservesData,
    address treasury,
    address asset
  ) external {
    DataTypes.ReserveData storage reserve = reservesData[asset];

    uint256 accruedToTreasuryShares = reserve.accruedToTreasuryShares;

    if (accruedToTreasuryShares != 0) {
      reserve.accruedToTreasuryShares = 0;
      uint256 normalizedIncome = reserve.getNormalizedIncome();
      uint256 amountToMint = accruedToTreasuryShares.rayMul(normalizedIncome);

++     // Update the interest rates before transferring to the treasury
++      reserve.updateInterestRates(
++        totalSupply,
++       DataTypes.ReserveCache({}), // Supply necessary cache data
++        asset,
++       IPool(asset).getReserveFactor(),
++        0, // No liquidity added
++       amountToMint, // Liquidity taken corresponds to amount sent to treasury
++        bytes32(0), // Position details (if any)
++       new bytes(0) // Interest rate data (if any)
++      );

      IERC20(asset).safeTransfer(treasury, amountToMint);
      totalSupply.supplyShares -= accruedToTreasuryShares;

      emit PoolEventsLib.MintedToTreasury(asset, amountToMint);
    }
  }

In this updated version, the interest rates are recalculated to account for the liquidity sent to the treasury. This ensures that the next user's deposit uses a correctly updated interest rate.

---

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

Honour commented:

Possibly valid. However claims the rates are inflated which i believe is false and the opposite happens( deflated rates) this is because the pool balance will be higher at the time interest rates are calculated (hence lower utilization and lower rates)

[nevillehuang]

request poc

[sherlock-admin4]

PoC requested from @Tomiwasa0

Requests remaining: 15

[Tomiwasa0]

1. After setting Flashloan premium to 0.09%

2. Import to the WithdrawtTEST

++ import {IPool} from './../../../../contracts/interfaces/pool/IPool.sol';
++ import {MockFlashLoanSimpleReceiver} from './../../../../contracts/mocks/MockSimpleFlashLoanReceiver.sol';

contract PoolWithdrawTests is PoolSetup {
++   address alice = address(1);
++  address bob = address(2);

  
++  event Transfer(address indexed from, address indexed to, uint256 value);

3. PASTE AND RUN THE POC

function _generateFlashloanCondition() internal {
    // Mint and approve tokenA and tokenC for bob
    _mintAndApprove(bob, tokenA, 60 ether, address(pool));
    _mintAndApprove(bob, tokenC, 2500 ether, address(pool));

    // Start prank as bob to simulate transactions from bob's account
    vm.startPrank(bob);

    // Supply tokenC to the pool for bob
    pool.supplySimple(address(tokenC), bob, 1000 ether, 0);

    // Stop prank as bob
    vm.stopPrank();
}

Updated testPoolWithdraw Function:

function testPoolWithdraw() external {
    // Declare amounts for supply, mint, withdraw, and borrow
    uint256 supplyAmount = 60 ether;
    uint256 mintAmount = 150 ether;
    uint256 withdrawAmount = 10 ether;
    uint256 index = 1;
    uint256 borrowAmount = 20 ether;

    // Mint and approve tokenA for owner
    vm.startPrank(owner);
    tokenA.mint(owner, mintAmount);
    tokenA.approve(address(pool), supplyAmount);

    // Supply tokenA to the pool for owner
    pool.supplySimple(address(tokenA), owner, supplyAmount, index);

    // Assert the balances after supplying tokenA
    assertEq(tokenA.balanceOf(address(pool)), supplyAmount, 'Pool Balance Supply');
    assertEq(tokenA.balanceOf(owner), mintAmount - supplyAmount, 'Owner Balance Supply');
    assertEq(pool.getTotalSupplyRaw(address(tokenA)).supplyShares, supplyAmount);
    assertEq(pool.getBalanceRaw(address(tokenA), owner, index).supplyShares, supplyAmount);

    // Advance time by 100 seconds
    uint256 currentTime1 = block.timestamp;
    vm.warp(currentTime1 + 100);

    // Borrow tokenA
    pool.borrowSimple(address(tokenA), owner, borrowAmount, 1);
    assertEq(tokenA.balanceOf(address(pool)), supplyAmount - borrowAmount);
    assertEq(pool.getDebt(address(tokenA), owner, 1), borrowAmount);
    assertEq(pool.totalDebt(address(tokenA)), borrowAmount);

    vm.stopPrank();

    // Advance time by 50 seconds
    uint256 currentTime2 = block.timestamp;
    vm.warp(currentTime2 + 50);

    // Prepare and execute flash loan
    bytes memory emptyParams;
    MockFlashLoanSimpleReceiver mockFlashSimpleReceiver = new MockFlashLoanSimpleReceiver(pool);
    _generateFlashloanCondition();

    uint256 premium = poolFactory.flashLoanPremiumToProtocol();

    vm.startPrank(alice);
    tokenA.mint(alice, 10 ether);

    // Expect flash loan event emission
    vm.expectEmit(true, true, true, true);
    emit PoolEventsLib.FlashLoan(address(mockFlashSimpleReceiver), alice, address(tokenA), 40 ether, (40 ether * premium) / 10_000);
    emit Transfer(address(0), address(mockFlashSimpleReceiver), (40 ether * premium) / 10_000);

    // Execute the flash loan
    pool.flashLoanSimple(address(mockFlashSimpleReceiver), address(tokenA), 40 ether, emptyParams);
    vm.stopPrank();

    // Advance time by 200 seconds
    uint256 currentTime = block.timestamp;
    vm.warp(currentTime + 200);

    // Assert the pool's balance after withdrawal
    assertEq(tokenA.balanceOf(address(pool)), 40036000000000000000, 'Pool Balance Withdraw');

    // Withdraw tokenA from the pool for the owner
    vm.startPrank(owner);
    pool.withdrawSimple(address(tokenA), owner, withdrawAmount, index);

    // Advance time by 50 seconds
    uint256 currentTime3 = block.timestamp;
    vm.warp(currentTime3 + 50);

    // Assert the remaining balance after withdrawal
    assertEq(pool.getBalanceRaw(address(tokenA), owner, index).supplyShares, 50000001310612529086);

    // Bob mints and supplies more tokenA
    vm.startPrank(bob);
    tokenA.mint(owner, mintAmount);
    tokenA.approve(address(pool), supplyAmount);
    pool.supplySimple(address(tokenA), bob, 60 ether, index);

    // Assert the balance after Bob's supply
    assertEq(pool.getBalanceRaw(address(tokenA), bob, index).supplyShares, 59999989872672182169);
}

Before Updating the index with Amount minted to tresury
Bob got - 59999989872672182169;
After update - 59999989869411349179,

Failing tests:
Encountered 1 failing test in test/forge/core/pool/PoolWithdrawTests.t.sol:PoolWithdrawTests
[FAIL. Reason: assertion failed: 59999989869411349179 != 59999989872672182169] testPoolWithdraw() (gas: 1531924)

Encountered a total of 1 failing tests, 0 tests succeeded

4. I agree with the initial statement that the impact is a deflation, Apologies for the confusion i calculated this on paper initially and a tiny error was made.
5. The attacker will mint more shares than they should and this can be weaponised to game the system for some profit by an attacker who just need to simply wait for a withdraw and then deposit lots of funds.
6. Since DefaultReserveInterestRateStrategy uses IERC20(params.reserve).balanceOf(msg.sender). Attacker gains more amount than they should when the new rate is normalised.