c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L26-L29

Vulnerability details

Impact

The protocol compounds interest on every call that updates the state. This is an intentional design choice. However, this does mean that the total return for the lender, and, conversely, the cost of debt for the borrower, can be influenced by the frequency of calls that update the state and by extension compound the interest.

The APR-based nature of the debt is made clear in the documentation, but what isn't explicitly stated is that for lenders to maximize their return they should endeavour to update the state through a call to WildcatMarket::updateState() weekly.

In the absence of a lender that regularly interacts with the protocol to compound interest, a borrower should aim to interact as little as possible with the protocol; this may be the difference between quarterly and weekly compounding.

This is stated in the whitepaper, but not in the Gitbook or other documentation (that the reviewer is aware of). The implication of this for the lender is not made clear. The below is the relevant excerpt from the whitepaper:

Fixed yield rates are chosen at deployment, specified in an APR that compounds each time someone interacts with the vault in a successful non-static call. Rates can be adjusted upwards by the borrower in order to incentivise lenders to participate (and downwards, subject to reserve ratio requirements),

As this can cause leakage of value (from the lender and protocol's perspective) if updateState() is not called regularly, this has been evaluated to medium.

Proof of Concept

The below PoC shows how two markets with the same characteristics can have different costs of debt to the borrower (and different returns for the lender).

The first case, test_borrowPoC has a weekly call to updateState. The second case, testBorrowPoCControl has a quarterly update.

Both these cases assume:

5 million total market supply
Interest at 1000 bps
Delinquency fee at 1000 bps
the borrower borrows 80% immediately and does not repay it back
240 weeks timeframe

Setup: In the TestConstants.sol file modify the DefaultMaximumSupply to:

uint128 constant DefaultMaximumSupply = 5_000_000e18;

Place the below code in the WildcatMarket.t.sol file.

The test can be run with forge test --match-test test_borrowPoC -vvv, this will run both the control and PoC tests.

  function test_borrowPoC() external {
    uint256 availableCollateral = market.borrowableAssets();
    assertEq(availableCollateral, 0, 'borrowable should be 0');

    vm.prank(alice);
    market.depositUpTo(5_000_000e18);
    assertEq(market.borrowableAssets(), 4000_000e18, 'borrowable should be 40k');
    vm.prank(borrower);
    market.borrow(4000_000e18);
    assertEq(asset.balanceOf(borrower), 4000_000e18);

    for (uint256 i; i < 240; ++i) {
          fastForward(1 weeks);
          market.updateState();
    }

    console.log("Scale factor: ", uint256(market.currentState().scaleFactor));
    console.log("Value to lender: ", market.balanceOf(alice));
    console.log("Protocol fees: ", market.currentState().accruedProtocolFees);
  }

  function test_borrowPoCControl() external {
    uint256 availableCollateral = market.borrowableAssets();
    assertEq(availableCollateral, 0, 'borrowable should be 0');

    vm.prank(alice);
    market.depositUpTo(500_0000e18);
    assertEq(market.borrowableAssets(), 4000_000e18, 'borrowable should be 40k');
    vm.prank(borrower);
    market.borrow(400_0000e18);
    assertEq(asset.balanceOf(borrower), 4000_000e18);

    for (uint256 i; i < 20; ++i) {
          fastForward(12 weeks);
          market.updateState();
    }

    console.log("Scale factor: ", uint256(market.currentState().scaleFactor));
    console.log("Value to lender: ", market.balanceOf(alice));
    console.log("Protocol fees: ", market.currentState().accruedProtocolFees);
  }

The output is:

Running 2 tests for test/market/WildcatMarket.t.sol:WildcatMarketTest
[PASS] test_borrowPoC() (gas: 12077205)
Logs:
  Scale factor:  2501444554515133233540074629
  Value to lender:  12507222772575666167700373
  Protocol fees:  375842179213842001906842

[PASS] test_borrowPoCControl() (gas: 1194476)
Logs:
  Scale factor:  2405453403328959908470687803
  Value to lender:  12027267016644799542353439
  Protocol fees:  357118397467353228856932

The weekly calls to compound the state results in a value of about 12.51 million tokens to the lender. The quarterly calls to compound the state results in a value of about 12.03 million to the lender.

There is roughly a 500_000e18 difference. It is also clear that the protocol accrues more fees with more regular calls that update the state.

We can thus conclude that if a lender/borrower does not interact with the WildcatMarket regularly, then less value will accrue to the lender and the protocol.

Note that the test values are chosen to demonstrate the concept.

Tools Used

Manual analysis.

Recommended Mitigation Steps

The sponsor noted that the compounding mechanism is made clear, which is true. Greater emphasis in terms of the consequences of neglecting to compound the interest regularly is recommended.

Unless the user is able to remain cognisant of the fact that state interaction equals compounding frequency, they might assume interest is compounding continuously/monthly/yearly by default as in TradFi.

Assessed type

Other

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #178

c4-judge commented 1 year ago

MarioPoneder marked the issue as unsatisfactory: Invalid

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

Interest accumulation linked to state updates may leak value #671