Fix: Fee recipient validation, rotation bypass, PostOnly cross-check mutation, free funds accounting, fee cap - #54
Open
yossweh wants to merge 1 commit into
Conversation
…s state mutation + MEDIUM-03 100% fee cap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes 1 HIGH and 3 MEDIUM severity vulnerabilities found during security audit of Phoenix V1 on-chain order book.
Vulnerabilities Fixed
HIGH-01: Fee Recipient Rotation Bypass (fees.rs)
Impact: Authority could change fee recipient without previous recipient signing if fees already collected, enabling fee diversion.
Fix: Force collect uncollected fees atomically before changing recipient.
MEDIUM-01: PostOnly Cross-Check Mutates State (fifo.rs)
Impact:
check_for_cross()auto-cancelled expired orders on opposite book during cross-check, emitting events and modifying state during read-only validation.Fix: Split into read-only
check_for_cross()+ explicitcleanup_expired_orders()instruction.MEDIUM-02: Free Funds Mode - Unlock Without Withdrawal (reduce_order.rs, fifo.rs)
Impact:
withdraw_funds=falseunlocked trader funds to "free" balance but didn't withdraw, leaving funds stuck in market vault.Fix: Ensure
claim_funds=truewhenever locked funds are unlocked.MEDIUM-03: 100% Taker Fee Allowed (initialize.rs)
Impact: Market creator could set
taker_fee_bps=10000(100%), draining all trade value to fees.Fix: Add reasonable cap at 1000 bps (10%) with governance override path.
Files Changed
program/processor/fees.rs- Force collect fees before recipient changestate/markets/fifo.rs- Read-only cross-check + explicit expired cleanupprogram/processor/reduce_order.rs- Require claim_funds when unlockingprogram/processor/initialize.rs- Cap taker fee at 1000 bpsTesting
Bug Bounty
Submitted per Phoenix Legacy Bug Bounty Program (up to $200K Critical / $25K High / $10K Medium / $5K Low).