fix(clob): enforce has_one on market vaults/mints in SettleFunds and PlaceOrder

Before this fix, SettleFunds and PlaceOrder only constrained the market's
fee_vault via has_one; they did not bind base_vault, quote_vault,
base_mint, or quote_mint to the addresses recorded on the Market PDA at
initialize_market time.

Because the fee_vault is a token account on the same mint and with the
same authority (market PDA) as the quote_vault, a caller with a positive
unsettled_quote balance could call SettleFunds and pass market.fee_vault
where quote_vault was expected. transfer_checked only verifies mint and
authority on the source account, not its identity, so the CPI would
succeed and drain accumulated taker fees to the attacker's own quote
ATA. The same gap existed on PlaceOrder for base_vault / quote_vault /
base_mint / quote_mint.

Fix: add has_one constraints on the market field in both SettleFunds and
PlaceOrder so Anchor rejects any mismatched address with ConstraintHasOne
(anchor error 2001) before any transfer runs. New error variants
(InvalidBaseVault, InvalidQuoteVault, InvalidBaseMint, InvalidQuoteMint)
mirror the existing InvalidFeeVault style so the failure is
self-describing.

Adds a LiteSVM regression test
(settle_funds_rejects_fee_vault_substituted_for_quote_vault) that earns
a buyer some unsettled_quote, then tries the attack and asserts the
transaction fails.

Author: Edward

defi/clob/anchor/programs/clob/src/errors.rs

@@ -41,6 +41,18 @@ pub enum ErrorCode {
     #[msg("Fee vault does not match the market's fee vault")]
     InvalidFeeVault,
 
+    #[msg("Base vault does not match the market's base vault")]
+    InvalidBaseVault,
+
+    #[msg("Quote vault does not match the market's quote vault")]
+    InvalidQuoteVault,
+
+    #[msg("Base mint does not match the market's base mint")]
+    InvalidBaseMint,
+
+    #[msg("Quote mint does not match the market's quote mint")]
+    InvalidQuoteMint,
+
     #[msg("Maker account provided does not correspond to a resting order on the book")]
     MakerAccountMismatch,
 

defi/clob/anchor/programs/clob/src/instructions/place_order.rs

@@ -422,9 +422,19 @@ pub fn handle_place_order<'info>(
 #[derive(Accounts)]
 #[instruction(side: OrderSide, price: u64, quantity: u64)]
 pub struct PlaceOrder<'info> {
+    // `has_one` ties every market-owned account on this struct to the
+    // addresses recorded on the Market PDA. Crucially, without
+    // has_one on base_vault / quote_vault / base_mint / quote_mint a caller
+    // could swap fee_vault in for quote_vault (same mint, same authority)
+    // and steer the per-fill fee transfer to drain real fees instead of
+    // routing them in.
     #[account(
         mut,
         has_one = fee_vault @ ErrorCode::InvalidFeeVault,
+        has_one = base_vault @ ErrorCode::InvalidBaseVault,
+        has_one = quote_vault @ ErrorCode::InvalidQuoteVault,
+        has_one = base_mint @ ErrorCode::InvalidBaseMint,
+        has_one = quote_mint @ ErrorCode::InvalidQuoteMint,
     )]
     pub market: Account<'info, Market>,
 

defi/clob/anchor/programs/clob/src/instructions/settle_funds.rs

@@ -3,6 +3,7 @@ use anchor_spl::token_interface::{
     transfer_checked, Mint, TokenAccount, TokenInterface, TransferChecked,
 };
 
+use crate::errors::ErrorCode;
 use crate::state::{Market, UserAccount, MARKET_SEED, USER_ACCOUNT_SEED};
 
 pub fn handle_settle_funds(context: Context<SettleFunds>) -> Result<()> {
@@ -64,7 +65,19 @@ pub fn handle_settle_funds(context: Context<SettleFunds>) -> Result<()> {
 
 #[derive(Accounts)]
 pub struct SettleFunds<'info> {
-    #[account(mut)]
+    // `has_one` constraints bind these vaults/mints to the addresses stored
+    // on the Market PDA at initialise_market time. Without them a caller
+    // could substitute the fee_vault (same mint + same authority as
+    // quote_vault) for `quote_vault` and drain accumulated taker fees,
+    // since transfer_checked only verifies mint + authority on the source
+    // account, not its identity.
+    #[account(
+        mut,
+        has_one = base_vault @ ErrorCode::InvalidBaseVault,
+        has_one = quote_vault @ ErrorCode::InvalidQuoteVault,
+        has_one = base_mint @ ErrorCode::InvalidBaseMint,
+        has_one = quote_mint @ ErrorCode::InvalidQuoteMint,
+    )]
     pub market: Account<'info, Market>,
 
     #[account(

defi/clob/anchor/programs/clob/tests/test_clob.rs

@@ -916,6 +916,83 @@ fn cancel_and_settle_bid_refunds_full_quote() {
     );
 }
 
+// Regression test for the fee-drain attack on settle_funds. Pre-fix,
+// `SettleFunds` did not bind `quote_vault` to `market.quote_vault` via
+// `has_one`, so a caller could pass `market.fee_vault` (same mint and
+// same authority) where `quote_vault` was expected and drain accumulated
+// taker fees while spending their own unsettled_quote credit. The
+// has_one constraint now bound on the `market` field must surface this
+// as `ConstraintHasOne` (anchor error 2001) before any transfer runs.
+#[test]
+fn settle_funds_rejects_fee_vault_substituted_for_quote_vault() {
+    let mut sc = full_setup();
+    initialize_market_and_users(&mut sc);
+
+    // Earn the buyer some unsettled_quote: place a bid (locks quote in
+    // the real quote_vault) and immediately cancel it (credits the
+    // buyer's unsettled_quote). settle_funds would normally drain the
+    // quote_vault to the buyer's ATA.
+    let bid_order_id = 1u64;
+    let place_ix = build_place_order_ix(
+        &sc,
+        &sc.buyer,
+        sc.buyer_user_account,
+        sc.buyer_base_ata,
+        sc.buyer_quote_ata,
+        clob::state::OrderSide::Bid,
+        bid_order_id,
+        BID_PRICE,
+        BID_QUANTITY,
+    );
+    let cancel_ix = build_cancel_order_ix(
+        &sc,
+        &sc.buyer.pubkey(),
+        sc.buyer_user_account,
+        bid_order_id,
+    );
+    send_transaction_from_instructions(
+        &mut sc.svm,
+        vec![place_ix, cancel_ix],
+        &[&sc.buyer],
+        &sc.buyer.pubkey(),
+    )
+    .unwrap();
+
+    // Build a settle_funds ix but swap fee_vault in for quote_vault.
+    // Everything else (base_vault, mints, user accounts, owner) stays
+    // correct, so the only thing that should reject this is the new
+    // has_one constraint on the market PDA.
+    let attack_ix = Instruction::new_with_bytes(
+        sc.program_id,
+        &clob::instruction::SettleFunds {}.data(),
+        clob::accounts::SettleFunds {
+            market: sc.market,
+            user_account: sc.buyer_user_account,
+            base_vault: sc.base_vault.pubkey(),
+            // Attack: route the quote-side transfer at the fee_vault.
+            quote_vault: sc.fee_vault.pubkey(),
+            user_base_account: sc.buyer_base_ata,
+            user_quote_account: sc.buyer_quote_ata,
+            base_mint: sc.base_mint,
+            quote_mint: sc.quote_mint,
+            owner: sc.buyer.pubkey(),
+            token_program: token_program_id(),
+        }
+        .to_account_metas(None),
+    );
+
+    let result = send_transaction_from_instructions(
+        &mut sc.svm,
+        vec![attack_ix],
+        &[&sc.buyer],
+        &sc.buyer.pubkey(),
+    );
+    assert!(
+        result.is_err(),
+        "settle_funds must reject fee_vault substituted for quote_vault"
+    );
+}
+
 #[test]
 fn initialize_market_rejects_zero_tick_size() {
     let mut sc = full_setup();