FixedTermLoanHooks Allows Borrower to Increase Loan Term

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/access/FixedTermLoanHooks.sol#L236

Vulnerability details

Summary

The FixedTermLoanHooks contract is designed to implement fixed-term loans within the Wildcat protocol. This contract allows a borrower to set a fixedTermEndTime during market creation, after which lenders are permitted to withdraw their funds. To provide flexibility, the borrower can update the fixedTermEndTime using the setFixedTermEndTime function.

The setFixedTermEndTime function in FixedTermLoanHooks lacks a constraint preventing the borrower from extending the loan term. While the function aims to allow the borrower to shorten the loan term, the current implementation does not explicitly prevent an increase in the fixedTermEndTime. This oversight permits the borrower to unilaterally extend the loan term, potentially trapping lenders' funds for longer than initially agreed upon.

Details

The setFixedTermEndTime function in FixedTermLoanHooks is intended to allow adjustments to the loan's fixed term. The function's logic checks if the proposed newFixedTermEndTime is greater than the current hookedMarket.fixedTermEndTime. If it is, the function reverts with the IncreaseFixedTerm error, preventing an extension of the term.

However, the function's logic is flawed. It should actually check if newFixedTermEndTime is less than the current hookedMarket.fixedTermEndTime to ensure that the term is only being shortened, not lengthened.

Impact

This vulnerability allows borrowers to change the loan terms in their favor after the market has been established.

Scenario

• A borrower deploys a Wildcat market using FixedTermLoanHooks, setting a fixedTermEndTime of 30 days from the deployment date.
• Lenders deposit funds into the market, expecting to be able to withdraw their funds after the 30-day fixed term.
• As the fixedTermEndTime approaches, the borrower calls the setFixedTermEndTime function, passing a new timestamp that is 60 days from the deployment date.
• Due to the lack of a proper check in the setFixedTermEndTime function, the loan term is extended to 60 days, and lenders are now unable to withdraw their funds until the new, extended deadline.

Fix

The solution is to correct the comparison in the setFixedTermEndTime function. Instead of checking if the new timestamp is greater than the current one, the function should check if the new timestamp is less than the existing one.

// FixedTermLoanHooks.sol

function setFixedTermEndTime(address market, uint32 newFixedTermEndTime) external onlyBorrower {
    HookedMarket storage hookedMarket = _hookedMarkets[market];
    if (!hookedMarket.isHooked) revert NotHookedMarket();

    // Corrected logic: Check if the new end time is earlier than the current one
    if (newFixedTermEndTime < hookedMarket.fixedTermEndTime) revert IncreaseFixedTerm();

    hookedMarket.fixedTermEndTime = newFixedTermEndTime;
    emit FixedTermUpdated(market, newFixedTermEndTime);
}

Assessed type

Invalid Validation

[d1ll0n]

I'm not sure what you mean about it lacking the restriction - it clearly does prevent extensions:

if (newFixedTermEndTime > hookedMarket.fixedTermEndTime) revert IncreaseFixedTerm();

Your proposed solution does the opposite of what you describe - reverting when the new term is less than the current term means borrowers can only increase the duration, which would allow them to indefinitely push back the date at which lenders can withdraw.

[3docSec]

Misunderstanding of the implementation

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid