From 21dca80f13eee60fc5831e01a872cb56283bb4dc Mon Sep 17 00:00:00 2001 From: Mike MacCana Date: Fri, 2 Oct 2026 14:41:51 +0000 Subject: [PATCH] Token Swap (Quasar): bind the reserves to the pool, re-read them after a swap deposit_liquidity, withdraw_liquidity, swap_tokens and claim_admin_fees took pool_a and pool_b with no check that they were the pool's reserves. A trader could name a mint-A token account of their own as pool_a: the swap priced itself from that account's one-unit balance, sent the input back into it, and paid out 9,999,989 of a 10,000,000 pool_b. PoolConfig now records both reserve addresses at initialize_pool, and the four handlers check them with has_one, failing with InvalidPoolVault. swap_rejects_substituted_pool_vault and deposit_rejects_substituted_pool_vaults run the attack. The Anchor v1 and v2 versions already bound the reserves as the pool's associated token accounts. swap_tokens also re-checked the constant-product invariant against reserves computed from the amounts it meant to move. It now reads both vaults after the transfers land, as the Anchor versions do. Claude-Session: https://claude.ai/code/session_01BvopS6WYzsT9Qf32NTu29V --- CHANGELOG.md | 14 +++ finance/token-swap/quasar/CHANGELOG.md | 18 ++++ finance/token-swap/quasar/src/error.rs | 3 + .../src/instructions/claim_admin_fees.rs | 6 ++ .../src/instructions/deposit_liquidity.rs | 6 +- .../src/instructions/initialize_pool.rs | 4 + .../quasar/src/instructions/swap_tokens.rs | 30 +++--- .../src/instructions/withdraw_liquidity.rs | 6 +- finance/token-swap/quasar/src/state.rs | 13 ++- finance/token-swap/quasar/src/tests.rs | 92 +++++++++++++++++++ 10 files changed, 174 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 307f8bfee..1077e13fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ All notable changes to this repository are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [2026-10-02] - Token Swap (Quasar): reserves are bound to the pool + +### Fixed + +- `finance/token-swap/quasar` accepted any token accounts as `pool_a` and + `pool_b` in `deposit_liquidity`, `withdraw_liquidity`, `swap_tokens` and + `claim_admin_fees`, so a trader could price a swap from a token account of + their own and drain the real reserve on the other side. `PoolConfig` now + records both reserves and every handler checks them (`InvalidPoolVault`). + The Anchor v1 and v2 versions already bound them as associated token + accounts of the pool. +- `finance/token-swap/quasar`'s `swap_tokens` re-checks the invariant against + the vault balances after its transfers, rather than against computed ones. + ## [2026-10-01] - Managed Fund's video script is removed ### Removed diff --git a/finance/token-swap/quasar/CHANGELOG.md b/finance/token-swap/quasar/CHANGELOG.md index 4e4fe8949..db00817dc 100644 --- a/finance/token-swap/quasar/CHANGELOG.md +++ b/finance/token-swap/quasar/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## [2026-10-02] + +### Fixed + +- `deposit_liquidity`, `withdraw_liquidity`, `swap_tokens` and + `claim_admin_fees` took `pool_a` and `pool_b` with no check that they were + the pool's reserves. A swap could name any mint-A token account the trader + owned as `pool_a`: the handler priced the trade from that account's balance, + sent the input into it, and paid out of the real `pool_b`. `PoolConfig` now + records both reserve addresses at `initialize_pool`, and the four handlers + check them with `has_one(pool_a)` and `has_one(pool_b)`, failing with the new + `InvalidPoolVault` error. `swap_rejects_substituted_pool_vault` and + `deposit_rejects_substituted_pool_vaults` run the attack. `PoolConfig` grows + by 64 bytes, so pools created before this change cannot be read by it. +- `swap_tokens` re-checked the constant-product invariant against reserves it + computed from the amounts it meant to transfer. It now reads both vaults' + balances after the transfers land, as the Anchor versions do. + ## [2026-09-22] ### Fixed diff --git a/finance/token-swap/quasar/src/error.rs b/finance/token-swap/quasar/src/error.rs index ddc971324..30e052c07 100644 --- a/finance/token-swap/quasar/src/error.rs +++ b/finance/token-swap/quasar/src/error.rs @@ -42,4 +42,7 @@ pub enum AmmError { MathOverflow, /// The signer of `claim_admin_fees` does not match `Config.admin`. Unauthorized, + /// A `pool_a` or `pool_b` account is not the reserve recorded on the + /// pool's `PoolConfig`. + InvalidPoolVault, } diff --git a/finance/token-swap/quasar/src/instructions/claim_admin_fees.rs b/finance/token-swap/quasar/src/instructions/claim_admin_fees.rs index 22e1e8ec9..ea61ec1f0 100644 --- a/finance/token-swap/quasar/src/instructions/claim_admin_fees.rs +++ b/finance/token-swap/quasar/src/instructions/claim_admin_fees.rs @@ -19,6 +19,8 @@ pub struct ClaimAdminFeesAccountConstraints { #[account( mut, address = PoolPda::seeds(config.address(), mint_a.address(), mint_b.address()), + has_one(pool_a) @ AmmError::InvalidPoolVault, + has_one(pool_b) @ AmmError::InvalidPoolVault, )] pub pool_config: Account, pub mint_a: Account, @@ -77,10 +79,14 @@ pub fn handle_claim_admin_fees( let config_addr = *accounts.pool_config.config(); let mint_a_addr = *accounts.pool_config.mint_a(); let mint_b_addr = *accounts.pool_config.mint_b(); + let pool_a_addr = *accounts.pool_config.pool_a(); + let pool_b_addr = *accounts.pool_config.pool_b(); accounts.pool_config.set_inner(PoolConfigInner { config: config_addr, mint_a: mint_a_addr, mint_b: mint_b_addr, + pool_a: pool_a_addr, + pool_b: pool_b_addr, admin_fees_owed_a: 0, admin_fees_owed_b: 0, }); diff --git a/finance/token-swap/quasar/src/instructions/deposit_liquidity.rs b/finance/token-swap/quasar/src/instructions/deposit_liquidity.rs index ad3f35612..a7611a3f2 100644 --- a/finance/token-swap/quasar/src/instructions/deposit_liquidity.rs +++ b/finance/token-swap/quasar/src/instructions/deposit_liquidity.rs @@ -16,7 +16,11 @@ pub struct DepositLiquidityAccountConstraints { #[account(address = ConfigPda::seeds())] pub config: Account, /// Owns both reserves and is the LP mint's authority; signs the mint_to. - #[account(address = PoolPda::seeds(config.address(), mint_a.address(), mint_b.address()))] + #[account( + address = PoolPda::seeds(config.address(), mint_a.address(), mint_b.address()), + has_one(pool_a) @ AmmError::InvalidPoolVault, + has_one(pool_b) @ AmmError::InvalidPoolVault, + )] pub pool_config: Account, /// Depositor (must be signer to authorise transfers). pub depositor: Signer, diff --git a/finance/token-swap/quasar/src/instructions/initialize_pool.rs b/finance/token-swap/quasar/src/instructions/initialize_pool.rs index 429b2541e..d0d127942 100644 --- a/finance/token-swap/quasar/src/instructions/initialize_pool.rs +++ b/finance/token-swap/quasar/src/instructions/initialize_pool.rs @@ -70,6 +70,10 @@ pub fn handle_initialize_pool( config: *accounts.config.address(), mint_a: *accounts.mint_a.address(), mint_b: *accounts.mint_b.address(), + // Recorded so every later handler can check the reserves it is + // handed are these two (`has_one(pool_a)`, `has_one(pool_b)`). + pool_a: *accounts.pool_a.address(), + pool_b: *accounts.pool_b.address(), // No swaps have happened yet, so the admin has no fee claim. These // accumulators are written by `swap_tokens` and zeroed by // `claim_admin_fees`. diff --git a/finance/token-swap/quasar/src/instructions/swap_tokens.rs b/finance/token-swap/quasar/src/instructions/swap_tokens.rs index f469b5c59..532a7d1e9 100644 --- a/finance/token-swap/quasar/src/instructions/swap_tokens.rs +++ b/finance/token-swap/quasar/src/instructions/swap_tokens.rs @@ -19,6 +19,8 @@ pub struct SwapTokensAccountConstraints { #[account( mut, address = PoolPda::seeds(config.address(), mint_a.address(), mint_b.address()), + has_one(pool_a) @ AmmError::InvalidPoolVault, + has_one(pool_b) @ AmmError::InvalidPoolVault, )] pub pool_config: Account, pub trader: Signer, @@ -171,10 +173,14 @@ pub fn handle_swap_tokens( let config_addr = *accounts.pool_config.config(); let mint_a_addr = *accounts.pool_config.mint_a(); let mint_b_addr = *accounts.pool_config.mint_b(); + let pool_a_addr = *accounts.pool_config.pool_a(); + let pool_b_addr = *accounts.pool_config.pool_b(); accounts.pool_config.set_inner(PoolConfigInner { config: config_addr, mint_a: mint_a_addr, mint_b: mint_b_addr, + pool_a: pool_a_addr, + pool_b: pool_b_addr, admin_fees_owed_a: new_owed_a, admin_fees_owed_b: new_owed_b, }); @@ -242,22 +248,18 @@ pub fn handle_swap_tokens( .invoke()?; } - // Verify invariant holds on the LP-claimable (effective) reserves. - // u128 + checked throughout - a raw `+`/`-` could wrap on extreme values. - let new_pool_a_raw = (pool_a_raw as u128) - .checked_add(if input_is_token_a { input as u128 } else { 0 }) - .ok_or(AmmError::MathOverflow)? - .checked_sub(if !input_is_token_a { output as u128 } else { 0 }) - .ok_or(AmmError::MathOverflow)?; - let new_pool_b_raw = (pool_b_raw as u128) - .checked_add(if !input_is_token_a { input as u128 } else { 0 }) - .ok_or(AmmError::MathOverflow)? - .checked_sub(if input_is_token_a { output as u128 } else { 0 }) - .ok_or(AmmError::MathOverflow)?; - let new_effective_a = new_pool_a_raw + // Verify invariant holds on the LP-claimable (effective) reserves, read + // from the vaults after the transfers have landed rather than computed + // from the amounts this handler meant to move. A check on computed figures + // only re-runs the math above; reading the vaults also catches a transfer + // that moved something other than what the math said. Quasar token + // accounts are zero-copy, so `amount()` reads the runtime buffer the CPIs + // just wrote and there is nothing to reload. + // u128 + checked throughout - a raw `-` could wrap on extreme values. + let new_effective_a = (accounts.pool_a.amount() as u128) .checked_sub(new_owed_a as u128) .ok_or(AmmError::MathOverflow)?; - let new_effective_b = new_pool_b_raw + let new_effective_b = (accounts.pool_b.amount() as u128) .checked_sub(new_owed_b as u128) .ok_or(AmmError::MathOverflow)?; let new_invariant = new_effective_a diff --git a/finance/token-swap/quasar/src/instructions/withdraw_liquidity.rs b/finance/token-swap/quasar/src/instructions/withdraw_liquidity.rs index 753ba6b58..fb9be32fa 100644 --- a/finance/token-swap/quasar/src/instructions/withdraw_liquidity.rs +++ b/finance/token-swap/quasar/src/instructions/withdraw_liquidity.rs @@ -14,7 +14,11 @@ pub struct WithdrawLiquidityAccountConstraints { #[account(address = ConfigPda::seeds())] pub config: Account, /// Owns both reserves and signs the transfers out of them. - #[account(address = PoolPda::seeds(config.address(), mint_a.address(), mint_b.address()))] + #[account( + address = PoolPda::seeds(config.address(), mint_a.address(), mint_b.address()), + has_one(pool_a) @ AmmError::InvalidPoolVault, + has_one(pool_b) @ AmmError::InvalidPoolVault, + )] pub pool_config: Account, pub depositor: Signer, /// LP mint at the LiquidityMintPda. diff --git a/finance/token-swap/quasar/src/state.rs b/finance/token-swap/quasar/src/state.rs index 59d15b082..89be5cbc7 100644 --- a/finance/token-swap/quasar/src/state.rs +++ b/finance/token-swap/quasar/src/state.rs @@ -25,8 +25,8 @@ pub struct Config { /// /// Holds the metadata that identifies a single pool: which `Config` it belongs /// to and which two mints it trades. The actual pool reserves live in separate -/// token accounts (`pool_a`, `pool_b`) that this account owns - they are not -/// stored here. This struct is the pool's *configuration*, not its state. +/// token accounts (`pool_a`, `pool_b`) that this account owns; their balances +/// are not stored here, only their addresses. This struct is the pool's *configuration*, not its state. /// /// This account is also the pool's signing authority: it owns both reserves, /// is the mint authority of the LP mint, and signs the transfers out of the @@ -46,6 +46,15 @@ pub struct PoolConfig { pub mint_a: Address, /// Mint of token B. pub mint_b: Address, + /// The pool's token A reserve, recorded by `initialize_pool`. Every + /// handler that touches the reserves checks the account it is handed + /// against this address (`has_one(pool_a)`), because a token account's + /// mint and authority do not identify it: anyone can create another + /// token account of mint A, and a swap that read its reserves from one + /// would price the trade from a balance the caller chose. + pub pool_a: Address, + /// The pool's token B reserve, recorded and checked as `pool_a` is. + pub pool_b: Address, /// Admin's accumulated fee claim on token A, in base units. Sits /// physically in `pool_a` but excluded from the LP curve and from /// LP-withdrawable amounts. Swept by `claim_admin_fees`. diff --git a/finance/token-swap/quasar/src/tests.rs b/finance/token-swap/quasar/src/tests.rs index 7afaec956..d8141567b 100644 --- a/finance/token-swap/quasar/src/tests.rs +++ b/finance/token-swap/quasar/src/tests.rs @@ -800,6 +800,98 @@ fn swap_slippage_rejected(test: &mut Test) { ); } +/// A swap that names a token account of its own as `pool_a` must be refused. +/// Without the `has_one(pool_a)` check the handler would price the trade from +/// that account's one-unit balance, send the trader's input into it (back to +/// the trader), and pay out nearly all of `pool_b` from the real reserve. +#[quasar_test] +fn swap_rejects_substituted_pool_vault(test: &mut Test) { + setup_pool(test); + seed_pool(test, 10_000_000, 10_000_000); + + test.add(Wallet::new().at(BAD_ACTOR)); + test.add( + TokenAccount::new(MINT_A, BAD_ACTOR) + .at(BAD_TOKEN_A) + .amount(1_000_001), + ); + // A second mint-A account the attacker owns, holding one unit, passed as + // the pool's token A reserve. + let fake_pool_a = Pubkey::new_from_array([25; 32]); + test.add( + TokenAccount::new(MINT_A, BAD_ACTOR) + .at(fake_pool_a) + .amount(1), + ); + + test.send(SwapTokensInstruction { + trader: BAD_ACTOR, + mint_a: MINT_A, + mint_b: MINT_B, + pool_a: fake_pool_a, + pool_b: POOL_B, + token_a: BAD_TOKEN_A, + token_b: BAD_TOKEN_B, + payer: PAYER, + input_is_token_a: true, + input_amount: 1_000_000, + min_output_amount: 1, + }) + .fails_with(AmmError::InvalidPoolVault); + + assert_eq!( + test.tokens(POOL_B), + 10_000_000, + "pool_b must be untouched after the refused swap" + ); +} + +/// A deposit that names the depositor's own token accounts as the reserves +/// must be refused. Without the `has_one` checks the LP tokens minted would be +/// priced from those accounts' balances, and the deposit would land in them. +#[quasar_test] +fn deposit_rejects_substituted_pool_vaults(test: &mut Test) { + setup_pool(test); + seed_pool(test, 10_000_000, 10_000_000); + + fund( + test, + BAD_ACTOR, + BAD_TOKEN_A, + BAD_TOKEN_B, + 1_000_000, + 1_000_000, + ); + let fake_pool_a = Pubkey::new_from_array([25; 32]); + let fake_pool_b = Pubkey::new_from_array([26; 32]); + test.add( + TokenAccount::new(MINT_A, BAD_ACTOR) + .at(fake_pool_a) + .amount(1), + ); + test.add( + TokenAccount::new(MINT_B, BAD_ACTOR) + .at(fake_pool_b) + .amount(1), + ); + + test.send(DepositLiquidityInstruction { + depositor: BAD_ACTOR, + mint_a: MINT_A, + mint_b: MINT_B, + pool_a: fake_pool_a, + pool_b: fake_pool_b, + liquidity_provider_token: Pubkey::new_from_array([27; 32]), + token_a: BAD_TOKEN_A, + token_b: BAD_TOKEN_B, + payer: PAYER, + amount_a: 1_000_000, + amount_b: 1_000_000, + minimum_lp_tokens_out: 0, + }) + .fails_with(AmmError::InvalidPoolVault); +} + // ─── claim_admin_fees ──────────────────────────────────────────────────────── #[quasar_test]