Skip to content

[SECURITY] Fix critical vault drain vulnerability in partial fill logic#1

Open
byte541 wants to merge 1 commit intoVK-RED:masterfrom
byte541:fix/partial-fill-event-amount
Open

[SECURITY] Fix critical vault drain vulnerability in partial fill logic#1
byte541 wants to merge 1 commit intoVK-RED:masterfrom
byte541:fix/partial-fill-event-amount

Conversation

@byte541
Copy link
Copy Markdown

@byte541 byte541 commented Feb 14, 2026

Summary

This PR fixes a critical security vulnerability that allows attackers to drain the market vault by exploiting incorrect amount recording in partial fill events.

Vulnerability Details

Severity: CRITICAL
Type: Vault Drain / Theft of Funds

When a maker's order is partially filled, the Fill event incorrectly records base_amount_to_set (the remaining unfilled amount) instead of the actual amount that was matched.

Vulnerable Code (before fix)

market_events.add_event(EventParams{
    base_amount: order.base_amount_to_set,  // BUG: This is REMAINING, not MATCHED
    ...
});

Impact

  1. Attacker places large Bid order (e.g., 10,000 tokens)
  2. Accomplice places small Ask order (e.g., 1 token)
  3. Partial fill: 1 token matched, but event records 9,999 (remaining)
  4. consume_events credits attacker with 9,999 tokens instead of 1
  5. Attacker settles and drains 9,999 tokens from vault

Fix

Added base_amount_matched field to track actual matched amount and use it in Fill events:

struct EditOrders {
    pub order_id: u64,
    pub base_amount_to_set: u64,
    pub base_amount_matched: u64,  // NEW: Track actual matched amount
    pub total_quote_amount: u64
}

// In event creation:
market_events.add_event(EventParams{
    base_amount: order.base_amount_matched,  // FIX: Use matched amount
    ...
});

Testing

I have prepared a proof-of-concept test demonstrating the vulnerability and fix. Happy to share if helpful.

Disclosure

This vulnerability was discovered during security research. I'm submitting this fix as part of responsible disclosure.


Found by byte541

CRITICAL SECURITY FIX

When a maker's order is partially filled, the Fill event was incorrectly
recording base_amount_to_set (the remaining unfilled amount) instead of
the actual amount that was matched.

This bug allows attackers to be credited more tokens than they're owed
when consume_events is called, enabling vault drain attacks.

Changes:
- Added base_amount_matched field to EditOrders struct
- Store base_amount_eaten when creating EditOrders for partial fills
- Use base_amount_matched in Fill event instead of base_amount_to_set

Impact: Prevents vault drain via inflated partial fill credits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant