code-423n4 / 2024-08-wildcat-findings

3 stars 1 forks source link

The `annualInterestBips` of a market hooked by a fixed-term hook can be reduced at any time even when the fixed term time of the market has not yet elapsed #95

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/access/FixedTermLoanHooks.sol#L960-L978

Vulnerability details

Impact

If a borrower reduces the APR of a market before its fixed term time elapses:

Proof of Concept

The Wildcat protocol specifies that a borrower can reduce the APR of their market as follows:

The interest rate on a market is fixed at any given point in time (i.e. markets do not make use of a utilisation-rate based curve), however the borrower is free to adjust this rate step-wise should they wish, under the following formula:

Should a borrower wish to increase the APR of a market in order to encourage additional deposits, they are able to do so without constraint.

Should they wish to decrease the APR, they are able to do so by up to 25% of the current APR in a given two week period: a decrease of more than this requires that twice the amount is returned to the market reserves for that two week period to permit lenders to opt out ('ragequit') if they choose.

To illustrate:

A borrower can reduce a market APR from 10% to 7.5% with no penalty, and two weeks thereafter will be able to reduce it again to 5.625%, and so on.

However, should a borrower reduce a market APR from 10% to 7.4% (a 26% reduction), they will be required to return 52% of the outstanding supply to the market for two weeks. After that time has passed, the reserve ratio will drop back to the prior level and the assets can be borrowed again.

Note that the above only applies if your market is in an 'open-term' setting: i.e. there is no hook enabled which is preventing withdrawals at the time of the proposed change. If this is the case, you will not be able to reduce the APR while that hook is active (otherwise that enables a fairly obvious rug mechanic).

However, if a borrower create a market hooked by a fixed-term hook, the borrower could reduce the APR to any value with no penalty while no lender can opt out.

Copy below codes to WildcatMarket.t.sol and run forge test --match-test test_setAnnualInterestAndReserveRatioBips_BeforeFixedTermExpired:

  function test_setAnnualInterestAndReserveRatioBips_BeforeFixedTermExpired() external {
    //@audit-info deploy a FixedTermLoanHooks template
    address fixedTermHookTemplate = LibStoredInitCode.deployInitCode(type(FixedTermLoanHooks).creationCode);
    hooksFactory.addHooksTemplate(
      fixedTermHookTemplate,
      'FixedTermLoanHooks',
      address(0),
      address(0),
      0,
      0
    );

    vm.startPrank(borrower);
    //@audit-info borrower deploy a FixedTermLoanHooks hookInstance
    address hooksInstance = hooksFactory.deployHooksInstance(fixedTermHookTemplate, '');
    DeployMarketInputs memory parameters = DeployMarketInputs({
      asset: address(asset),
      namePrefix: 'name',
      symbolPrefix: 'symbol',
      maxTotalSupply: type(uint128).max,
      annualInterestBips: 1000,
      delinquencyFeeBips: 1000,
      withdrawalBatchDuration: 10000,
      reserveRatioBips: 1000,
      delinquencyGracePeriod: 10000,
      hooks: EmptyHooksConfig.setHooksAddress(address(hooksInstance))
    });
    //@audit-info borrower deploy a market hooked by a FixedTermLoanHooks hookInstance
    address market = hooksFactory.deployMarket(
      parameters,
      abi.encode(block.timestamp + (365 days)),
      bytes32(uint(1)),
      address(0),
      0
    );
    vm.stopPrank();
    //@audit-info lenders can only withdraw their asset one year later
    assertEq(FixedTermLoanHooks(hooksInstance).getHookedMarket(market).fixedTermEndTime, block.timestamp + (365 days));
    //@audit-info alice deposit 50K asset into market
    vm.startPrank(alice);
    asset.approve(market, type(uint).max);
    WildcatMarket(market).depositUpTo(50_000e18);
    vm.stopPrank();
    //@audit-info current annualInterestBips is 10%
    assertEq(WildcatMarket(market).annualInterestBips(), 1000);
    //@audit-info current reserveRatioBips is 10%
    assertEq(WildcatMarket(market).reserveRatioBips(), 1000);
    //@audit-info borrower reduces annualInterestBips from 10% to 6%
    vm.prank(borrower);
    WildcatMarket(market).setAnnualInterestAndReserveRatioBips(600, 0);
    assertEq(WildcatMarket(market).annualInterestBips(), 600);
    //@audit-info reserveRatioBips is updated to 80% (2 * (10%-6%)/10%)
    assertEq(WildcatMarket(market).reserveRatioBips(), 8000);
    //@audit-info however, no lender can opt-out due to the fixed term has not expired
    vm.expectRevert(FixedTermLoanHooks.WithdrawBeforeTermEnd.selector);
    vm.prank(alice);
    WildcatMarket(market).queueWithdrawal(10_000e18);    
    fastForward(2 weeks);
    //@audit-info 2 weeks later, the borrower can lock annualInterestBips at 6% and restore reserveRatioBips to 10%
    vm.prank(borrower);
    WildcatMarket(market).setAnnualInterestAndReserveRatioBips(600, 0);
    assertEq(WildcatMarket(market).reserveRatioBips(), 1000);
  }

Tools Used

Manual review

Recommended Mitigation Steps

The annualInterestBips of a market hooked by a fixed-term hook should not be allowed to be reduced before the fixed-term time has elapsed:

  function onSetAnnualInterestAndReserveRatioBips(
    uint16 annualInterestBips,
    uint16 reserveRatioBips,
    MarketState calldata intermediateState,
    bytes calldata hooksData
  )
    public
    virtual
    override
    returns (uint16 updatedAnnualInterestBips, uint16 updatedReserveRatioBips)
  {
+   HookedMarket memory market = _hookedMarkets[msg.sender];
+   if (!market.isHooked) revert NotHookedMarket();
+   if ((annualInterestBips < intermediateState.annualInterestBips) && (market.fixedTermEndTime > block.timestamp)) {
+     revert ReduceAPRBeforeTermEnd();
+   }
    return
      super.onSetAnnualInterestAndReserveRatioBips(
        annualInterestBips,
        reserveRatioBips,
        intermediateState,
        hooksData
      );
  }

Assessed type

Context

c4-judge commented 1 month ago

3docSec changed the severity to 2 (Med Risk)

c4-judge commented 1 month ago

3docSec marked the issue as satisfactory