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

3 stars 1 forks source link

`FixedTermLoanHooks` allow Borrower to update Annual Interest before end of the "Fixed Term Period" #77

Open howlbot-integration[bot] opened 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#L857-L859 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/access/FixedTermLoanHooks.sol#L960-L978

Vulnerability details

Summary

While the documentation states that in case of 'fixed term' market the APR cannot be changed until the term ends, nothing prevents this in FixedTermLoanHooks.

Vulnerability details

In Wildcat markets, lenders know in advance how much APR the borrower will pay them. In order to allow lenders to exit the market swiftly, the market must always have at least a reserve ratio of the lender funds ready to be withdrawn. If the borrower decide to reduce the APR, in order to allow lenders to 'ragequit', a new reserve ratio is calculated based on the variation of the APR as described in the link above. Finally, is a market implement a fixed term (date until when withdrawals are not possible), it shouldn't be able to reduce the APR, as this would allow the borrower to 'rug' the lenders by reducing the APR to 0% while they couldn't do anything against that.

The issue here is that while lenders are (as expected) prevented to withdraw before end of term: https://github.com/code-423n4/2024-08-wildcat/blob/main/src/access/FixedTermLoanHooks.sol#L857-L859

File: src/access/FixedTermLoanHooks.sol
848:   function onQueueWithdrawal(
849:     address lender,
850:     uint32 /* expiry */,
851:     uint /* scaledAmount */,
852:     MarketState calldata /* state */,
853:     bytes calldata hooksData
854:   ) external override {
855:     HookedMarket memory market = _hookedMarkets[msg.sender];
856:     if (!market.isHooked) revert NotHookedMarket();
857:     if (market.fixedTermEndTime > block.timestamp) {
858:       revert WithdrawBeforeTermEnd();
859:     }

this is not the case for the borrower setting the annual interest: https://github.com/code-423n4/2024-08-wildcat/blob/main/src/access/FixedTermLoanHooks.sol#L960-L978

File: src/access/FixedTermLoanHooks.sol
960:   function onSetAnnualInterestAndReserveRatioBips(
961:     uint16 annualInterestBips,
962:     uint16 reserveRatioBips,
963:     MarketState calldata intermediateState,
964:     bytes calldata hooksData
965:   )
966:     public
967:     virtual
968:     override
969:     returns (uint16 updatedAnnualInterestBips, uint16 updatedReserveRatioBips)
970:   {
971:     return
972:       super.onSetAnnualInterestAndReserveRatioBips(
973:         annualInterestBips,
974:         reserveRatioBips,
975:         intermediateState,
976:         hooksData
977:       );
978:   }
979: 

Impact

Borrower can rug the lenders by reducing the APR while they cannot quit the market

Proof of Concept

Add this test to test/access/FixedTermLoanHooks.t.sol

  function testAudit_SetAnnualInterestBeforeTermEnd() external {
    DeployMarketInputs memory inputs;

    // "deploying" a market with MockFixedTermLoanHooks
    inputs.hooks = EmptyHooksConfig.setHooksAddress(address(hooks));
    hooks.onCreateMarket(
        address(this),              // deployer
        address(1),             // dummy market address
        inputs,                 // ...
        abi.encode(block.timestamp + 365 days, 0) // fixedTermEndTime: 1 year, minimumDeposit: 0
    );

    vm.prank(address(1));
    MarketState memory state;
    // as the fixedTermEndTime isn't past yet, it's not possible to withdraw
    vm.expectRevert(FixedTermLoanHooks.WithdrawBeforeTermEnd.selector);
    hooks.onQueueWithdrawal(address(1), 0, 1, state, '');

    // but it is still possible to reduce the APR to zero
    hooks.onSetAnnualInterestAndReserveRatioBips(0, 0, state, "");
  }

Tools Used

Manual review

Recommended Mitigation Steps

When FixedTermLoanHooks::onSetAnnualInterestAndReserveRatioBips is called, revert if market.fixedTermEndTime > block.timestamp

Assessed type

Invalid Validation

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 selected for report