supplyIndex will be arbitrarily inflated due to vulnerable implementation in calculateLinearInterest and _updateSupplyIndex

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/libraries/logic/InterestLogic.sol#L267 https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/libraries/math/MathUtils.sol#L33

Vulnerability details

Impact

supplyIndex will be arbitrarily inflated due to vulnerable implementation in calculateLinearInterest() and _updateSupplyIndex().

Proof of Concept

Unlike borrowIndex, supplyIndex uses a linear increase formula (rate * deltaT).

The issue is implementation in calculateLinearInterest() and _updateSupplyIndex() is incorrect, and will arbitrarily compound supplyIndex depending on how often _updateSupplyIndex() is invoked in borrow, repay, liquidation flows.

calculateLinearInterest() adds WadRayMath.RAY to the calculated result, effectively turning it into a multiplier that compounds the interest whenever rayMul is used in _updateSupplyIndex().

//src/libraries/math/MathUtils.sol
  function calculateLinearInterest(
    uint256 rate,
    uint256 lastUpdateTimestamp,
    uint256 currentTimestamp
  ) internal pure returns (uint256) {
    //solium-disable-next-line
    uint256 result = rate * (currentTimestamp - lastUpdateTimestamp);
    unchecked {
      result = result / SECONDS_PER_YEAR;
    }

|>    return WadRayMath.RAY + result;
  }

(https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/libraries/math/MathUtils.sol#L33)

Each time _updateSupplyIndex() is invoked, the interest is compounded rather than linearly increased.

//src/libraries/logic/InterestLogic.sol
  function _updateSupplyIndex(DataTypes.AssetData storage assetData) internal {
    // Only cumulating on the supply side if there is any income being produced
    // The case of Fee Factor 100% is not a problem (supplyRate == 0),
    // as liquidity index should not be updated
    if (assetData.supplyRate != 0) {
      uint256 cumulatedSupplyInterest = MathUtils.calculateLinearInterest(
        assetData.supplyRate,
        assetData.lastUpdateTimestamp
      );
|>      uint256 nextSupplyIndex = cumulatedSupplyInterest.rayMul(assetData.supplyIndex);
      assetData.supplyIndex = nextSupplyIndex.toUint128();
    }
  }

(https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/libraries/logic/InterestLogic.sol#L267)

POC: suppose during the same time delta, a total of 6% linear distribution ( calculateLinearInterest() returns (1 + 6%) ). supplyIndex starts from 1 (leaving out precision scaling for simplicity):

Case A: _updateSupplyIndex() is invoked three times during this time delta with each step linear increase 2% ( at each time calculateLinearInterest() returns (1 + 2%) ).

supplyIndex after = 1 x (1 + 0.02)^3 -> 1.061208

Case B: _updateSupplyIndex() is invoked once at the end of this time delta with total linear increase of 6%.

supplyIndex after = 1 x (1 + 0.06) -> 1.06

Current implementation inadvertently compounds the supply interest when it should only be applying a linear increase. This results in an arbitrarily inflated supplyIndex value depending on the frequency of atomic _updateSupplyIndex() calls.

Tools Used

Manual

Recommended Mitigation Steps

(1) In calculateLinearInterest, Consider simply returning result without adding RAY; (2) In _updateSupplyIndex, supplyIndex should be updated by adding cumulatedSupplyInterest directly, scaled appropriately, ensuring a linear increase rather than compounding;

Assessed type

Other

[MarioPoneder]

Seen issues of this kind in many protocols. Has always been intended business logic / accepted side-effect of compounding so far.
Invalidating for now but open for further input from @thorseldon and Warden.

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Invalid