c4-submissions commented 10 months ago

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

c4-pre-sort commented 10 months ago

minhquanym marked the issue as low quality report

c4-pre-sort commented 10 months ago

minhquanym marked the issue as sufficient quality report

minhquanym commented 10 months ago

Consider QA

laurenceday commented 10 months ago

Willing to acknowledge as QA.

c4-sponsor commented 10 months ago

laurenceday (sponsor) acknowledged

c4-sponsor commented 10 months ago

laurenceday marked the issue as disagree with severity

c4-judge commented 10 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

MarioPoneder commented 10 months ago

This is a good feature suggestion to account for paused tokens.
However, the users the decide which tokens they want to use with this protocol, therefore it's a risk they have to bear and not a bug of the protocol per se.