sherlock-audit / 2024-06-new-scope-judging

1 stars 1 forks source link

neon2835 - PoolGetter's Calculation of User or Pool Balance and Debt is Not Real-Time, it may lead to severe and dangerous situations #503

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

neon2835

High

PoolGetter's Calculation of User or Pool Balance and Debt is Not Real-Time, it may lead to severe and dangerous situations

Summary

In PoolGetter, the balance and debt calculations for users or pools in the following functions are not real-time values:

  1. function getBalanceByPosition(address asset, bytes32 positionId) external view returns (uint256 balance)
  2. function getBalance(address asset, address who, uint256 index) external view returns (uint256 balance)
  3. function totalAssets(address asset) external view returns (uint256 balance)
  4. function totalDebt(address asset) external view returns (uint256 balance)
  5. function getDebtByPosition(address asset, bytes32 positionId) external view returns (uint256 debt)
  6. function getDebt(address asset, address who, uint256 index) external view returns (uint256 debt)
  7. function supplyAssets(address asset, bytes32 positionId) external view returns (uint256)
  8. function debtAssets(address asset, bytes32 positionId) external view returns (uint256)

When external functions call these functions to query data, if they require real-time values, it may lead to severe and dangerous situations.

Vulnerability Detail

The main reason for the above issues is that they ignore the fact that both the balance and debt values can change over time. In practice, a pool may not update its liquidityIndex or borrowIndex for a period of time, as these values are only updated when functions like supply, borrow, withdraw, repay, liquidation, or forceUpdateReserves are triggered.

Let’s assume the pool’s liquidityRate and borrowRate are both non-zero, and the liquidityIndex and borrowIndex have not been updated for an hour. In this case, the functions querying the balance and debt should take time into account in the calculation formulas. Otherwise, the values retrieved will not be real-time. This poses significant risks for upstream functions that rely on these values if they do not account for this issue.

Impact

The balance and debt calculations in PoolGetter for users or pools are not real-time, which can lead to critical errors in upstream logic. For example, one observed issue is: The balance and debt calculations in PoolGetter for users or pools are not real-time, which can lead to critical errors in upstream logic. For example, one observed issue is:

  1. UIHelper contract(The display value of the UI interface will may be inconsistent with the actual situation): https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/periphery/ui-helpers/UIHelper.sol#L215-L216
  2. CuratedVault:

Code Snippet

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/PoolGetters.sol#L34

Tool used

Manual Review

Recommendation

Taking the getBalance function as an example, the original code is:

  function getBalance(address asset, address who, uint256 index) external view returns (uint256 balance) {
    bytes32 positionId = who.getPositionId(index);
    return _balances[asset][positionId].getSupplyBalance(_reserves[asset].liquidityIndex);
  }

Optimized version:

  function getBalance(address asset, address who, uint256 index) external view returns (uint256 balance) {
    bytes32 positionId = who.getPositionId(index);
    // Dynamic calculation
    uint128 currLiquidityIndex = _reserves[asset].liquidityIndex;
    uint128 currLiquidityRate = _reserves[asset].liquidityRate;
    uint40 lastUpdateTimestamp = _reserves[asset].lastUpdateTimestamp;
    uint256 cumulatedLiquidityInterest = MathUtils.calculateLinearInterest(currLiquidityRate, lastUpdateTimestamp);
    uint128 nextLiquidityIndex = cumulatedLiquidityInterest.rayMul(uint256(currLiquidityIndex)).toUint128();
    // now nextLiquidityIndex is the newest
    return _balances[asset][positionId].getSupplyBalance(nextLiquidityIndex);
  }

Duplicate of #473