The Wildcat Protocol - Fulum's results

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L142 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L134

Vulnerability details

Impact

It's impossible to set a new max total supply or to close a market on an existing market.

Proof of Concept

The two functions WildcatMarketConfig::setMaxTotalSupply() and WildcatMarket::closeMarket() have the onlyController modifier to restrict the access of theses methods to only WildcatMarketController can call them. But if we check in the WildcatMarketController contract, no function or arbitrary call can call theses functions. And because only the controller contract can call them, the two functions are unusable. It lead to an impossibility to update the maximum total supply of the market and even worse to close a market. It impossible for the borrower to restrict the deposit method / entry point to potential lenders inside in his market.

Tools Used

Manual Review

Recommended Mitigation Steps

Add these two functions on WildcatMarketController contract

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketToken.sol#L64-L82 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L163-L187

Vulnerability details

Impact

A lender can transfer his tokens before any potential call to nukeFromOrbit() to avoid an escrow to be created with his funds and keep them even if he is sanctionned. He can make bad operations without fear of his tokens being escrowed.

Proof of Concept

Check this scenario:

1. Bob, a lender, has lent some DAI in the "West Ham Capital" market and has equivalent in WHCDAI. He has two wallets (Wallet1 & Wallet2) of lenders accepted in the market.
2. Bob make a bad action and he is sanctionned by the Chainalysis oracle and now he risks having his funds confiscated by a potential call of WildcatMarketConfig::nukeFromOrbit().
3. Bob create a bot and monitor any call of nukeFromOrbit() with his address to frontrun it with a WildcatMarketToken::transfer() to transfer his WHCDAI to his second wallet (Wallet2). - He can simply make a transfer() when he know his address is blacklisted by the Chainalysis oracle but not yet in the market.
4. nukeFromOrbit() is executed and because the Bob Wallet1.scaledBalance is now equal to zero, AuthRole.Blocked is set for the Wallet1, but escrow is not created and the sanctionned lenders keep all his funds but now on the Wallet2 and he can continue to take action on the market or withdraw.

Check the _blockAccount() function to better understand:

    function _blockAccount(MarketState memory state, address accountAddress) internal {
        Account memory account = _accounts[accountAddress];
        if (account.approval != AuthRole.Blocked) {
            uint104 scaledBalance = account.scaledBalance;
            account.approval = AuthRole.Blocked;
            emit AuthorizationStatusUpdated(accountAddress, AuthRole.Blocked);

            if (scaledBalance > 0) {
                account.scaledBalance = 0;
                address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
                    accountAddress,
                    borrower,
                    address(this)
                );
                emit Transfer(accountAddress, escrow, state.normalizeAmount(scaledBalance));
                _accounts[escrow].scaledBalance += scaledBalance;
                emit SanctionedAccountAssetsSentToEscrow(
                    accountAddress,
                    escrow,
                    state.normalizeAmount(scaledBalance)
                );
            }
            _accounts[accountAddress] = account;
        }
    }

I don't know the exact scenarios which a lenders can be sanctionned by the Chainalysis oracle. But it's a possible problematic operation because if a lender have two registered wallets on the market (consciously or unconsciously for the borrower). He can potentially try to make bad operations with one address without fear of being sanctionned by transfering his claimable power to his second address.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a verification step into the custom transfer() and transferFrom() functions in WildcatMarketToken to verify if a lender is sanctionned and prevent him to transfer his tokens when it's the case.

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/libraries/FeeMath.sol#L97-L111

Vulnerability details

Impact

Because some ERC20 tokens can be paused. In this case, the state.timeDelinquent is still updated and an unfair APR penalty for the borrower may be added when the market is in delinquent state.

Proof of Concept

If a lender decide to use WildcatMarketWithdrawals::queueWithdrawal(), he can make the market delinquent in sending the reserve ratio of the market below it's delinquent threshold. Like it's explained below in the docs: https://wildcat-protocol.gitbook.io/wildcat/using-wildcat/delinquency#unclaimed-pending-withdrawals-and-delinquency

"The reserve ratio of a market can be temporarily raised depending on the amount of assets that are either earmarked for withdrawal by a lender, or are part of a pending withdrawal request. ... "

Now the market is in delinquent state and when the reserve ratio is not sufficient, the borrower have to send tokens in the market to return in a non-delinquent state. When the market is in delinquent state, the borrower have a grace period, it's a certain amount of time to return funds to the market before the penalty APR (additional interest rate penalty) must be applied and penalize the borrower.

The problem is a large number often used ERC20 tokens have functionnality to pause for preventing all interactions with the contract in case of problems/hack. It's the case for the USDT token contract, USDC, PAXG, WBTC, etc. If an ERC20 tokens used in collateral for Wildcat Market is paused, no transactions is permitted with it. Therefore the borrower can't send back tokens to the market and stop the delinquent state of the market. But during the time the ERC20 is paused, time goes by and impact the borrower by potentially get the market beyonf the grace period and adding penaltyAPR to the market. Even more if the borrower have set a very low grace period to boost lenders confidence in his market.

Tools Used

Manual Review

Recommended Mitigation Steps

You can implement a function to inform the code that the asset contract is paused. A potential way to make it is to add a function inside the WildcatArchController that can be triggered manually by the owner (a borrower or lender can't have power on this feature) to specify the a specific asset is paused. Then in FeeMath::updateTimeDelinquentAndGetPenaltyTime(), verify the paused status on WildcatArchController and prevent the update of state.timeDelinquent if the ERC20 token of the contract is paused.

This is a sensitive element that can be difficult to add to the code, but it's important to guarantee the borrower that he won't pay unjustified penalty.

Assessed type

ERC20