code-423n4 / 2023-10-wildcat-findings

12 stars 9 forks source link

New approved lender can receive other peoples accrued interest fees #692

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/libraries/FeeMath.sol#L142-L174

Vulnerability details

A new approved Lender by the borrower, getting into the market at the right time can make huge profits in the market due to activity in the market of others, and accruing interest, which make the lender withdraw immediately, without being in the market for a long time or as long as others in the market have being.

Proof of Concept

Lets explore how this will work

The market has being created for a while and people have being participating in the market and everything has being progressing as usual,

Then the borrower adds the LENDER, the LENDER is looking to make huge cash gains and fast studies how the market, and spots an opportunity,

The opportunity comes, in the form of the Protocol using a scaled system to calculate accurately interest accrual for all participants in the market, so their initial deposit will also benefit from any interest accrual that happens in the market, so their rebasing market token will also reflect that profit

This interesting system Updates the scale factors from time to time, to try to keep updates on how much a participant in the system,

function _getUpdatedState() internal returns (MarketState memory state) {
    state = _state;
    // Handle expired withdrawal batch
    if (state.hasPendingExpiredBatch()) {
      uint256 expiry = state.pendingWithdrawalExpiry;
      // Only accrue interest if time has passed since last update.
      // This will only be false if withdrawalBatchDuration is 0.
      if (expiry != state.lastInterestAccruedTimestamp) {
        (uint256 baseInterestRay, uint256 delinquencyFeeRay, uint256 protocolFee) = state
          .updateScaleFactorAndFees(
            protocolFeeBips,
            delinquencyFeeBips,
            delinquencyGracePeriod,
            expiry
          );
        emit ScaleFactorUpdated(state.scaleFactor, baseInterestRay, delinquencyFeeRay, protocolFee);
      }
      _processExpiredWithdrawalBatch(state);
    }

The vulnerability exploit can happen in the form, when the state is about to get updated in the Market Base Contract, the LENDER sees an action that the state is about to get updated, then they get involved, why ? Because they know that in the process of updating the state the scale factor also gets increased based on occurring actions in the market based on factors like base interest fee and the delinquency fee, Which will make the scale factor increase.

function updateScaleFactorAndFees(
    MarketState memory state,
    uint256 protocolFeeBips,
    uint256 delinquencyFeeBips,
    uint256 delinquencyGracePeriod,
    uint256 timestamp
  )
    internal
    pure
    returns (uint256 baseInterestRay, uint256 delinquencyFeeRay, uint256 protocolFee)
  {
    baseInterestRay = state.calculateBaseInterest(timestamp);

    if (protocolFeeBips > 0) {
      protocolFee = state.applyProtocolFee(baseInterestRay, protocolFeeBips);
    }

    if (delinquencyFeeBips > 0) {
      delinquencyFeeRay = state.updateDelinquency(
        timestamp,
        delinquencyFeeBips,
        delinquencyGracePeriod
      );
    }

    // Calculate new scaleFactor
    uint256 prevScaleFactor = state.scaleFactor;
    uint256 scaleFactorDelta = prevScaleFactor.rayMul(baseInterestRay + delinquencyFeeRay);

    state.scaleFactor = (prevScaleFactor + scaleFactorDelta).toUint112();
    state.lastInterestAccruedTimestamp = uint32(timestamp);
  }

Knowing that the scaleFactor is about to increase, the LENDER wants to deposit their funds before this update. If they can do this, their deposit would be at the old scaleFactor, which is lower. To ensure their transaction gets priority, the LENDER sends a deposit transaction with a higher gas price. This makes it more attractive to miners, increasing the chances of it getting confirmed before the scaleFactor update transaction.

Right after the LENDER’S deposit, the transaction to update the scaleFactor gets confirmed. This increases the scaleFactor due to the newly accrued interest. LENDER can now initiate a withdrawal. When they do, they would be withdrawing their original deposit along with the additional interest they shouldn't have earned. This is because their deposit got associated with the old scaleFactor just moments before it was updated. Effectively, the LENDER managed to earn interest instantly, an advantage they gained through frontrunning.

Tools Used

VS Code, Foundry and Manual Analysis

Recommended Mitigation Steps

The issue comes in the form of the scale factor system used in accounting for how much a user should receive in interest based on their initially deposited funds into the market, so the system would have to be evaluated to factor things like new lenders coming in and claiming rewards instantly based on how the maths for the interest is done in the protocol.

Assessed type

MEV

c4-pre-sort commented 10 months ago

minhquanym marked the issue as low quality report

minhquanym commented 10 months ago

Insufficient proof

c4-judge commented 10 months ago

MarioPoneder marked the issue as unsatisfactory: Insufficient proof