Lender can achieve higher APR than configured via daily compounding

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L26-L29 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L358 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/libraries/FeeMath.sol#L153-L172 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/libraries/FeeMath.sol#L23-L27 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/libraries/FeeMath.sol#L45-L50

Vulnerability details

Impact

Lenders can initiate compounding through the WildcatMarket#updateState() function in the protocol. Daily compounding by a lender can exceed the configured APR, resulting in higher than expected loan costs for the borrower and increased protocol fees.

Proof of Concept

The current protocol implementation allows anyone to invoke WildcatMarket#updateState(). This function calculates and updates interest rates and applies accrued protocol fees.

Interest rate calculation logic resides in FeeMath#updateScaleFactorAndFees(). This logic compounds interest rates and fees since the last invocation.

The formula for scaleFactorDelta is:

$scaleFactorDelta = prevScaleFactor \times (baseInterestRay + delinquencyFeeRay)$

Protocol fees are also compounded using logic in FeeMath#applyProtocolFees(). The formula for protocolFee is:

$protocolFee = scaledTotalSupply \times scaleFactor \times protocolFeeRay$

The interest rate and fee calculation logic is designed to compound changes since the last calculation every time it's called, giving lenders the flexibility to choose their compounding frequency.

An illustrative example:

• Borrower creates a market with the following params:
  • APR = 10%
  • Protocol Fee (%) = 20% (equivalent to 2% of APR)
  • Max Deposity Supply = 10,000,000 USDC
• Lender deposits 10,000,000 USDC
• The assets remain in the market for a year.

It's a well known fact that compounding frequency plays a significant role in determining the final amount of compound interest.

The formula for calculating the interest is: $$A = P \times (1 + \frac{r}{n})^{nt}$$ where:

$A$ - Future value of the invested amount $P$ - Principal $r$ - Interest rate $n$ - Number of times interest is compounded $t$ - Number of years

For daily and annually compounding, the formulas are respectively:

$$A{annually} = \$10,000,000 \times (1 + \frac{0.1}{1})^{1 \times 1}$$ $$A{daily} = \$10,000,000 \times (1 + \frac{0.1}{365})^{365 \times 1}$$

In the given example:

$$A{annually} = \$11,000,000.00$$ $$A{daily} \approx \$11,051,557.82$$

Daily compounding results in an APR of 10.52%, an unexpected additional 0.52% for the borrower.

Over longer periods, this issue becomes more pronounced, especially for large investments by organizations like market makers.

For instance, for the above example market a 5-year period with daily compounding yields:

• a 3.81% higher ROI (or 380 983.70 USDC more in absolute terms)
• an extra 38,098.37 USDC in protocol fees;

To demonstrate this issue, a POC test is provided. It highlights the difference in compounding between daily and annual scenarios.

Create a file named LenderCanAchieveHigherAPR.t.sol and place it in the test/ folder:

// SPDX-License-Identifier: MIT
pragma solidity >=0.8.20;

//import 'src/interfaces/IMarketEventsAndErrors.sol';
import 'src/libraries/MathUtils.sol';
import 'src/libraries/MarketState.sol';
import './BaseMarketTest.sol';

import "forge-std/console2.sol";

contract LenderCanAchieveHigherAPRTest is BaseMarketTest {
  uint256 constant public NUM_OF_YEARS = 1;

  function test_compoundDailyVsAnnually() external {
    parameters.namePrefix = "Tricky Lender ";
    parameters.symbolPrefix = "TRICKYLENDER";
    parameters.maxTotalSupply = 10_000_000 * 1e18;

    deployMarket(parameters);

    _approve(alice, address(market), type(uint256).max);
    vm.prank(alice);
    market.depositUpTo(10_000_000 ether);

    uint256 snapshot = vm.snapshot();

    MarketState memory annually = compoundAnnually();

    vm.revertTo(snapshot);

    MarketState memory daily = compoundDaily();

    console2.log("------------------- Year %s Stats (Compound annually) -------------------", NUM_OF_YEARS);
    console2.log("Scaled Total Supply (Annually compounding):    %s", annually.scaledTotalSupply);
    console2.log("Scaled Total Supply (Daily compounding):       %s", daily.scaledTotalSupply);
    console2.log("-------------------------------------------------------------------------");
    console2.log("Scale Factor (Annually compounding):           %s", annually.scaleFactor);
    console2.log("Scale Factor (Daily compounding):              %s", daily.scaleFactor);
    console2.log("-------------------------------------------------------------------------");
    console2.log("Accrued Protocol Fees (Annually compounding):   %s", annually.accruedProtocolFees);
    console2.log("Accrued Protocol Fees (Daily compounding):      %s", daily.accruedProtocolFees);
  }

  function compoundAnnually() internal returns (MarketState memory mktState) {
    for (uint i = 1; i <= NUM_OF_YEARS; i++) {
      fastForward(365 days);
      market.updateState();
    }
    mktState = market.previousState();
  }

  function compoundDaily() internal returns (MarketState memory mktState) {
    for (uint i = 1; i <= NUM_OF_YEARS * 365; i++) {
      fastForward(1 days);
      market.updateState();
    }
    mktState = market.previousState();
  }
}

Run the test like that:

forge test --match-path ./test/LenderCanAchieveHigherAPR.t.sol -vvv

This is what the output looks like:

------------------- Year 5 Stats (Compound annually) -------------------
  Scaled Total Supply (Annually compounding):    10000000000000000000000000
  Scaled Total Supply (Daily compounding):       10000000000000000000000000
  -------------------------------------------------------------------------
  Scale Factor (Annually compounding):           1610510000000000000000000000
  Scale Factor (Daily compounding):              1648608369073065848042335380
  -------------------------------------------------------------------------
  Accrued Protocol Fees (Annually compounding):   610510000000000000000000
  Accrued Protocol Fees (Daily compounding):      648608369073065848042344

Tools Used

Manual code review Foundry tests

Recommended Mitigation Steps

Revise the logic governing interest rate and protocol fee calculations by transitioning to an annual compounding model. Subsequently, adjust the calculation to determine a fraction of the compounding interest rate based on the specified interval (delta) for which interest rates are calculated.

Assessed type

Math

[laurenceday]

This is expected behaviour, and the reason that the rate is specified in APR rather than APY.

All documentation surrounding the protocol is very clear that the actual rate is variable, compounding on each non-static call. If we used the term APY instead, this would be a valid finding. As it stands, it is not.

Observing the difference between annual and daily compounding in a market like this is not reasonable. It would make more sense to check the difference between say, monthly and every block. Even so, the delta is effectively negligible (and again, accepted given the way that rates have been defined):

Finally, this issue is not even in scope as it was identified by alpeh_v in her previous review: see the last note:

"Lender interest rates are compounded based on the transaction frequency and therefore have a strong dependency on the number of transactions, the differential between per block compounding (aprox every 12 seconds) vs weekly or monthly can be large over the period. In addition when there is a completed batch the interest from the period between the last call and expiry will be overcharged by the amount of the interest accumulated after the block is completed because in this case the interest accumulation function is called twice once up to expiry and once after."

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-pre-sort]

minhquanym marked the issue as sufficient quality report

[c4-sponsor]

laurenceday (sponsor) disputed

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Invalid

[MarioPoneder]

Agree with the sponsor, the compounding is expected and intentional.
Therefore, this and related submissions are invalid.
Nevertheless, I appreciate the quality of some sumbmissions around this topic. Was a good read!