Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Users are incentivized to not withdraw immediately after the market is closed. #121

Open
howlbot-integration bot opened this issue Oct 3, 2024 · 4 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-01 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_14_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarketBase.sol#L665

Vulnerability details

Impact

Users are incentivized to not withdraw immediately after market is closed.

Proof of Concept

Within a withdraw batch, all users within said batch are paid equally - at the same rate, despite what exactly was the rate when each individual one created their withdraw.

While this usually is not a problem as it is a way to reward users who queue the withdrawal and start the expiry cooldown, it creates a problematic situation when the market is closed with an outstanding expiry batch.

The problem is that up until the expiry timestamp comes, all new withdraw requests are added to this old batch where the rate of the previous requests drags the overall withdraw rate down.

Consider the following scenario:

  1. A withdraw batch is created and its expiry time is 1 year.
  2. 6 months in, the withdraw batch has half of the markets value in it and the market is closed. The current rate is 1.12 and the batch is currently filled at 1.06
  3. Now users have two choices - to either withdraw their funds now at ~1.06 rate or wait 6 months to be able to withdraw their funds at 1.12 rate.

This creates a very unpleasant situation as the users have an incentive to hold their funds within the contract, despite not providing any value.

Looked from slightly different POV, these early withdraw requesters force everyone else to lock their funds for additional 6 months, for the APY they should've usually received for just holding up until now.

Tools Used

Manual review

Recommended Mitigation Steps

After closing a market and filling the current expiry, delete it from pendingWithdrawalExpiry. Introduce a closedExpiry variable so you later make sure a future expiry is not made at that same timestamp to avoid collision.

Assessed type

Context

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_14_group AI based duplicate group recommendation 🤖_primary AI based primary recommendation bug Something isn't working primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Oct 3, 2024
howlbot-integration bot added a commit that referenced this issue Oct 3, 2024
@d1ll0n
Copy link

d1ll0n commented Oct 9, 2024

Thanks for this, good find! Will adopt the proposed solution and see if it fixes #64

@d1ll0n d1ll0n added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Oct 9, 2024
@c4-judge
Copy link
Contributor

3docSec marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Oct 10, 2024
@c4-judge
Copy link
Contributor

3docSec marked the issue as selected for report

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Oct 10, 2024
@laurenceday
Copy link

Fixed with wildcat-finance/v2-protocol@b25e528

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-01 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_14_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants