Excessive Interest and Fees Due to Multiple Repayments

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L202

Vulnerability details

Vulnerability Detail

The Wildcat protocol design stipulates that borrowers should pay lenders an agreed-upon Annual Percentage Rate (APR). For instance, if a lender deposits 100,000 worth of an asset with a 10% APR and a protocol fee of 10% of the APR, the lender should receive 111,000 worth of the asset after one year (365 days).

This design assumes that the borrower repays the loan in a single transaction at the end of the loan period. However, if the borrower decides to repay the loan in multiple installments, they will pay more interest and fees than expected. This occurs because each time the borrower calls the repay function, the accrued interest and protocol fees are calculated. Therefore, subsequent repayments include interest on the previously accrued interest, resulting in the borrower paying a higher total than initially agreed upon.

The more the number of repayments, the more the borrower will pay.

  function repay(uint256 amount) external nonReentrant sphereXGuardExternal {
    if (amount == 0) revert_NullRepayAmount();

    asset.safeTransferFrom(msg.sender, address(this), amount);
    emit_DebtRepaid(msg.sender, amount);

@>   MarketState memory state = _getUpdatedState();
    if (state.isClosed) revert_RepayToClosedMarket();

    // Execute repay hook if enabled
    hooks.onRepay(amount, state, _runtimeConstant(0x24));

    _writeState(state);
  }

function _getUpdatedState() internal returns (MarketState memory state) {
    state = _state;
    // Handle expired withdrawal batch
    if (state.hasPendingExpiredBatch()) {
      console.log("1");
      uint256 expiry = state.pendingWithdrawalExpiry;
      // Only accrue interest if time has passed since last update.
      // This will only be false if withdrawalBatchDuration is 0.
      uint32 lastInterestAccruedTimestamp = state.lastInterestAccruedTimestamp;
      if (expiry != lastInterestAccruedTimestamp) {
        (uint256 baseInterestRay, uint256 delinquencyFeeRay, uint256 protocolFee) = state
          .updateScaleFactorAndFees(
            delinquencyFeeBips,
            delinquencyGracePeriod,
            expiry
          );
        emit_InterestAndFeesAccrued(
          lastInterestAccruedTimestamp,
          expiry,
          state.scaleFactor,
          baseInterestRay,
          delinquencyFeeRay,
          protocolFee
        );
      }
      _processExpiredWithdrawalBatch(state);
    }
    uint32 lastInterestAccruedTimestamp = state.lastInterestAccruedTimestamp;
    // Apply interest and fees accrued since last update (expiry or previous tx)
    if (block.timestamp != lastInterestAccruedTimestamp) {
@>      (uint256 baseInterestRay, uint256 delinquencyFeeRay, uint256 protocolFee) @>  state
@>       .updateScaleFactorAndFees(
@>         delinquencyFeeBips,
@>          delinquencyGracePeriod,
@>          block.timestamp
@>       );
      emit_InterestAndFeesAccrued(
        lastInterestAccruedTimestamp,
        block.timestamp,
        state.scaleFactor,
        baseInterestRay,
        delinquencyFeeRay,
        protocolFee
      );
    }

    // If there is a pending withdrawal batch which is not fully paid off, set aside
    // up to the available liquidity for that batch.
    if (state.pendingWithdrawalExpiry != 0) {
       console.log("3");
      uint32 expiry = state.pendingWithdrawalExpiry;
      WithdrawalBatch memory batch = _withdrawalData.batches[expiry];
      if (batch.scaledAmountBurned < batch.scaledTotalAmount) {
        // Burn as much of the withdrawal batch as possible with available liquidity.
        uint256 availableLiquidity = batch.availableLiquidityForPendingBatch(state, totalAssets());
        if (availableLiquidity > 0) {
          _applyWithdrawalBatchPayment(batch, state, expiry, availableLiquidity);
          _withdrawalData.batches[expiry] = batch;
        }
      }
    }
  }

Tools

Manual Review

Impact

Overpayment by Borrowers: Borrowers will incur significantly more fees and interest if they repay the loan in multiple installments rather than in a single payment.

Unintended Debt Accumulation: The debt accrual mechanism results in interest compounding on already accrued interest, which is not part of the intended loan terms.

Proof of Concept

function test_more_fees() public {

    asset.mint(josh, 100000 ether);
    asset.mint(borrower,  11000 ether);

    vm.startPrank(josh);
    market.deposit(100000 ether);
    vm.stopPrank();

    vm.startPrank(borrower);
    market.borrow(100000 ether);
    fastForward(360 days);
    asset.approve(address(market),  type(uint256).max);
    market.repay(50000 ether);
    vm.stopPrank();

    // At this point debt has moved from 100000000000000000000000 to 110849315068493150684931
    console.log("debt", market.totalDebts()); //  110849315068493150684931

    vm.startPrank(borrower);
    fastForward(5 days);
    market.repay(61000 ether);
    vm.stopPrank();

    // At this point debt has moved from 110849315068493150684931 to 111014862075436291987239
    console.log("debt", market.totalDebts()); //  111014862075436291987239

    // if borrower tries to close the market it will fail because the borrower needs to pay the remaining funds
    // eventhough the borrower has paid the actual APR + Protocol fees
    vm.startPrank(borrower);
    vm.expectRevert();
    market.closeMarket();
    vm.stopPrank();

  }

Recommended Mitigation Steps

The implementation should only be implemented such that making repayment multiple times does not lead to borrowers paying more fees and interest.

Assessed type

Other

[laurenceday]

Expected behaviour, the interest rate is specifically described in documentation as updating after every non-static call.

In any event, it isn't as severe as you might imagine, we looked into this last year. Absent someone poking it every single block, this is effectively void.

[3docSec]

Deliberate design choice with an accepted discrepancy between continuous and discrete compounding

[c4-judge]

3docSec marked the issue as unsatisfactory: Insufficient proof