Reserved assets for withdrawals can be lower than they should

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarket.sol#L59 https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarketBase.sol#L430

Vulnerability details

Impact

Deposited assets that should be reserved for withdrawals are not properly allocated, causing the withdrawal assets to be lower than they should be.

Description

When a market is in a delinquent state (indicating no obligatory assets available), and there are pending withdrawal requests, the assets available at the time are reserved for withdrawal.

However, due to the way the code is currently structured, the assets reserved for withdrawal may end up being lower than they should , causing the withdrawal to remain in the withdrawal queue instead of moving to the unclaimed withdrawals pool.

Lenders use the deposit function to deposit assets into the protocol. These newly deposited assets can also be used to cure delinquency, ensuring there are enough assets in the system. As a result, the borrower may not need to return assets immediately, and these deposits can also be used to fulfil withdrawal requests by other lenders. However, due to the current implementation of the code, the assets reserved for withdrawal may be less than they should be.

That happens because when the lender deposits, the function to procced the withdrawal requests is being called at the beginning of the function rather than when the assets are available in the market, this will make the assets be lower than they should when the next flow returns, potentially removing the delinquency fees and other accrued interest which have already been paid for that same block and timestamp at the beginning of the call.

In the current implementation, the getUpdatedState function, which processes withdrawal requests, is called at the beginning of the deposit function but not after the assets have been fully deposited and available for withdrawals.

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarket.sol#L59

58:     // Get current state
59:>    MarketState memory state = _getUpdatedState();

61:     if (state.isClosed) revert_DepositToClosedMarket();

         // Reduce amount if it would exceed totalSupply
64:     amount = MathUtils.min(amount, state.maximumDeposit());

Consider the following scenario:

• There are 100 assets in the market.
• When the deposit function is called to deposit an additional 100 assets, interest is accrued—let's assume it's 10 assets. This leaves 90 assets available for withdrawal requests (if any exist).

In this case, the 100 newly deposited assets should have been reserved for withdrawal, but because interest was accrued at the start of the transaction (the next flow), the actual available assets for withdrawal drop to 80 instead of 100.

Tools Used

Manual review

Recommended Mitigation Steps

The following code should be utilized to mitigate the issue

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarket.sol#L86

 // Transfer deposit from caller

        asset.safeTransferFrom(msg.sender, address(this), amount);

        account.scaledBalance += scaledAmount;

        _accounts[msg.sender] = account;

        emit_Transfer(_runtimeConstant(address(0)), msg.sender, amount);

        emit_Deposit(msg.sender, amount, scaledAmount);

        // Increase supply

        state.scaledTotalSupply += scaledAmount;

>   if (state.hasPendingExpiredBatch()) {
        _processExpiredWithdrawalBatch(state);

            }

        // Update stored state

        _writeState(state);

        return amount;

    }

This ensures that any pending withdrawal requests are processed first, preventing the newly deposited assets from being reserved for other purposes before the pending withdrawals are fully handled. This helps maintain a fair and accurate flow of assets within the protocol

Assessed type

Context

[d1ll0n]

This is primarily an issue of gas and codesize. In order to get the current scale factor, which we need to know in order to calculate the actual amount to transfer and deposit, we need to have updated the state to the current block. To count the deposited amount toward the existing withdrawals, we'd need to redo the batch handling part of the state update a second time, which would be costly.

[c4-judge]

3docSec marked the issue as satisfactory

[c4-judge]

3docSec marked the issue as selected for report

[c4-judge]

3docSec marked the issue as not selected for report

[c4-judge]

3docSec marked the issue as duplicate of #62