Malicious Lender Can Block Other Lenders from Withdrawing Funds

[c4-bot-6]

Lines of code

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

Vulnerability details

Summary

The Wildcat protocol allows borrowers to integrate with external hooks to enforce custom logic for their markets. The AccessControlHooks and FixedTermLoanHooks contracts are examples of such hooks, providing access control and fixed-term loan functionalities. Both hooks use the isKnownLenderOnMarket mapping to track if a lender has made their first deposit into a market. This mapping is updated in the _writeLenderStatus function, which is called after a successful deposit or when specific conditions are met during transfer or withdrawal operations.

A malicious lender can manipulate the isKnownLenderOnMarket mapping by sending a small amount of tokens to another address (address B) during their first deposit transaction. This action marks address B as a "known lender" even though they have not initiated the deposit. Consequently, address B might not possess a valid credential or be eligible for withdrawals, but the system mistakenly recognizes them as a known lender, creating discrepancies in withdrawal eligibility and potentially blocking legitimate withdrawals for address B.

Details

The _writeLenderStatus function in AccessControlHooks and FixedTermLoanHooks updates the isKnownLenderOnMarket mapping. The function has a canSetKnownLender argument that dictates whether the lender should be marked as known. This argument is set to true for deposits and under specific circumstances in the onTransfer and onQueueWithdrawal functions, particularly when a lender is not already known and has a valid credential.

The onTransfer function in AccessControlHooks and FixedTermLoanHooks allows a known lender to transfer market tokens to any address without restrictions. This means a lender making their first deposit can transfer a small amount to another address (address B) within the same transaction.

Due to the transfer happening within the first deposit transaction, address B gets marked as a known lender in the _writeLenderStatus function. However, address B might not have initiated a deposit nor possess a valid credential required for withdrawal.

The onQueueWithdrawal function in FixedTermLoanHooks allows withdrawals if the lender is a known lender or if they have a valid credential. If the malicious lender transferred a small amount to address B during their first deposit, address B is incorrectly recognized as a known lender, potentially allowing them to bypass credential checks during withdrawal, even if they are ineligible.

Impact

This bug enables a malicious lender to manipulate the system's recognition of known lenders. It can lead to:

Incorrect Withdrawal Eligibility: Addresses that haven't made a deposit might be wrongly permitted to queue withdrawals.

Blocked Withdrawals: If address B attempts to withdraw without a valid credential, relying on their incorrect status as a "known lender", their withdrawal request will be blocked. This is because the system, although incorrectly marking them as known, will not find a valid credential for them, ultimately preventing the withdrawal.

Scenario

• A malicious lender (address A) creates a new market with either AccessControlHooks or FixedTermLoanHooks as the hook contract.
• Address A makes their first deposit into the market, and within the same transaction, transfers a small amount to another address (address B).
• The _writeLenderStatus function incorrectly marks address B as a "known lender," even though they didn't initiate the deposit.
  Address B, now falsely recognized as a known lender, attempts to queue a withdrawal.
• The system, unable to find a valid credential for address B, blocks the withdrawal, even though address B is considered a "known lender."

Fix

To prevent this vulnerability:

Modify the _writeLenderStatus function: Instead of setting canSetKnownLender to true for all deposit-related calls, make it more restrictive. Only mark an address as a known lender if the call is made directly from the onDeposit function. This ensures that only genuine first-time depositors are marked as known lenders.

// AccessControlHooks.sol and FixedTermLoanHooks.sol

function _writeLenderStatus(
    LenderStatus memory status,
    address accountAddress,
    bool hasValidCredential,
    bool wasUpdated,
    bool canSetKnownLender
) internal {

    // ... Existing code ...

    // Mark account as a known lender if they have a valid credential, are not
    // already known, and the function WAS CALLED FROM onDeposit.
    if (
        canSetKnownLender.and(hasValidCredential).and(
            msg.sender == address(this)).and( // Check if the caller is the contract itself
            !isKnownLenderOnMarket[accountAddress][msg.sender]
        )
    ) {
        isKnownLenderOnMarket[accountAddress][msg.sender] = true;
        emit AccountMadeFirstDeposit(accountAddress);
    }

    // ... Existing code ...
}

(Optional) Add Validation to onTransfer: Implement a check in the onTransfer function within both hook contracts to prevent known lenders from transferring tokens during their first deposit if the recipient is not already a known lender. This could be achieved by verifying if the sender's balance is zero before processing the transfer.

Assessed type

Context