Skip to content

Security Audit: 5 Medium Findings with Fixes#288

Open
AdeshAtole wants to merge 1 commit intoopenbook-dex:masterfrom
AdeshAtole:audit/security-fixes
Open

Security Audit: 5 Medium Findings with Fixes#288
AdeshAtole wants to merge 1 commit intoopenbook-dex:masterfrom
AdeshAtole:audit/security-fixes

Conversation

@AdeshAtole
Copy link

Security Audit Report

Findings (all Medium severity)

1. Fees from place_take_order permanently locked in vault

  • File: instructions/place_take_order.rs:51
  • referrer_amount is added to fees_accrued but never to fees_available, making it unsweepable
  • Fix: Route referrer_amount to fees_available in place_take_order

2. Missing owner check on Deposit allows griefing

  • File: accounts_ix/deposit.rs
  • Anyone can deposit tokens into any OOA (no is_owner_or_delegate check)
  • Prevents OOA closure, locks victim's rent SOL
  • Fix: Add is_owner_or_delegate constraint

3. DecrementTake self-trade pollutes event heap

  • File: state/orderbook/book.rs:141-155
  • Self-trades with DecrementTake still generate fill events, allowing heap flooding
  • Informational fix suggested in report (no code change — requires design decision)

4. Full event heap panics all taker orders (DoS)

  • File: state/orderbook/book.rs:206 (push_back asserts !full)
  • When heap is full, ALL new matching orders fail
  • Fix: Graceful check before matching to stop taking when heap is full

5. execute_maker slot reuse race condition

  • File: state/orderbook/book.rs:294, open_orders_account.rs:279
  • Fill events reference maker slots by index; if slot is reused before crank, wrong order is affected
  • Informational — requires order-id validation in execute_maker (no code change — complex fix)

Code fixes applied for findings 1, 2, and 4.

Full report in AUDIT_REPORT.md.

1. [Medium] place_take_order fees permanently locked in vault - route referrer_amount to fees_available
2. [Medium] Missing owner check on Deposit allows griefing - add is_owner_or_delegate constraint
3. [Medium] Event heap full causes panic on taker orders - add graceful check before matching

Full audit report in AUDIT_REPORT.md with 5 findings total.
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