From 958004fa9d84b6a5e062fa190388391c4b8abede Mon Sep 17 00:00:00 2001 From: Aung Nanda Oo Date: Thu, 20 Aug 2026 18:53:36 -0700 Subject: [PATCH 1/3] security: fix swapped transfer amounts + broken invariant check in token-swap swap_exact_tokens_for_tokens's swap_a=false branch (B->A swaps) had two independent bugs in the same ~20-line block. Neither was ever exercised by tests - the only existing swap test always calls swap_a=true. 1. The two token transfers in the swap_a=false branch were swapped: the pool paid out `input` (the raw, caller-chosen B amount, reused as if it were the A payout) instead of the formula-computed `output`, and the trader only paid `output` instead of the full `input` they claimed to be swapping. Hand-verified this drains real value: on a pool with pool_a=1,000,000/pool_b=4,000,000 (5% fee), input=500,000 lets a trader pull out half the pool's entire A reserve while paying only ~106,145 B - the invariant drops from 4e12 to ~2.05e12 in one call. 2. The post-trade invariant check compared pool_account_a.amount against itself (A*A) instead of against pool_account_b.amount (A*B), so it never actually verified the real constant-product invariant. These two bugs currently mask each other by coincidence: severe drains via bug 1 always shrink pool_account_a a lot, and shrinking A a lot also shrinks A^2 a lot, so bug 2's check - wrong as it is - still happens to catch the most extreme drains. A more moderate trade can still be profitably exploitable while slipping under that coincidental threshold, and fixing either bug alone (without the other) would remove the accidental protection and make the remaining bug fully exploitable. Both are fixed together here. Also switched the pre/post-trade invariant computation and the fee calculation to u128 arithmetic, matching the checked-math style already used for the output-amount calculation in the same function - the raw u64 multiplications could otherwise overflow and panic (overflow-checks = true in this workspace's release profile) for large-supply pools. Added 'Swap from B to A' to tests/swap.ts, the first test in this example to exercise that branch at all. Verified it fails against the unpatched code (reverts with InvariantViolated, since the trade genuinely destroys value under the old code) and passes after the fix, with all other existing tests unaffected. --- .../swap_exact_tokens_for_tokens.rs | 17 ++++++---- tokens/token-swap/anchor/tests/swap.ts | 32 +++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs b/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs index ede88cf5d..b80747146 100644 --- a/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs +++ b/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs @@ -25,9 +25,11 @@ pub fn swap_exact_tokens_for_tokens( input_amount }; - // Apply trading fee, used to compute the output + // Apply trading fee, used to compute the output. + // u128 avoids overflow when input * fee approaches u64::MAX. let amm = &ctx.accounts.amm; - let taxed_input = input - input * amm.fee as u64 / 10000; + let fee_amount = ((input as u128) * (amm.fee as u128) / 10000) as u64; + let taxed_input = input - fee_amount; let pool_a = &ctx.accounts.pool_account_a; let pool_b = &ctx.accounts.pool_account_b; @@ -51,8 +53,8 @@ pub fn swap_exact_tokens_for_tokens( return err!(TutorialError::OutputTooSmall); } - // Compute the invariant before the trade - let invariant = pool_a.amount * pool_b.amount; + // Compute the invariant before the trade (u128 to avoid overflow on large pools) + let invariant = (pool_a.amount as u128) * (pool_b.amount as u128); // Transfer tokens to the pool let authority_bump = ctx.bumps.pool_authority; @@ -99,7 +101,7 @@ pub fn swap_exact_tokens_for_tokens( }, signer_seeds, ), - input, + output, )?; token::transfer( CpiContext::new( @@ -110,7 +112,7 @@ pub fn swap_exact_tokens_for_tokens( authority: ctx.accounts.trader.to_account_info(), }, ), - output, + input, )?; } @@ -126,7 +128,8 @@ pub fn swap_exact_tokens_for_tokens( // We tolerate if the new invariant is higher because it means a rounding error for LPs ctx.accounts.pool_account_a.reload()?; ctx.accounts.pool_account_b.reload()?; - if invariant > ctx.accounts.pool_account_a.amount * ctx.accounts.pool_account_a.amount { + let new_invariant = (ctx.accounts.pool_account_a.amount as u128) * (ctx.accounts.pool_account_b.amount as u128); + if invariant > new_invariant { return err!(TutorialError::InvariantViolated); } diff --git a/tokens/token-swap/anchor/tests/swap.ts b/tokens/token-swap/anchor/tests/swap.ts index f809ac5f0..7a724dc12 100644 --- a/tokens/token-swap/anchor/tests/swap.ts +++ b/tokens/token-swap/anchor/tests/swap.ts @@ -92,4 +92,36 @@ describe('Swap', () => { values.defaultSupply.sub(values.depositAmountB).add(input).toNumber(), ); }); + + it('Swap from B to A', async () => { + const input = new anchor.BN(3 * 10 ** 6); + await program.methods + .swapExactTokensForTokens(false, input, new anchor.BN(100)) + .accountsPartial({ + amm: values.ammKey, + pool: values.poolKey, + poolAuthority: values.poolAuthority, + trader: values.admin.publicKey, + mintA: values.mintAKeypair.publicKey, + mintB: values.mintBKeypair.publicKey, + poolAccountA: values.poolAccountA, + poolAccountB: values.poolAccountB, + traderAccountA: values.holderAccountA, + traderAccountB: values.holderAccountB, + }) + .signers([values.admin]) + .rpc({ skipPreflight: true }); + + const traderTokenAccountA = await connection.getTokenAccountBalance(values.holderAccountA); + const traderTokenAccountB = await connection.getTokenAccountBalance(values.holderAccountB); + expect(traderTokenAccountB.value.amount).to.equal( + values.defaultSupply.sub(values.depositAmountB).sub(input).toString(), + ); + expect(Number(traderTokenAccountA.value.amount)).to.be.greaterThan( + values.defaultSupply.sub(values.depositAmountA).toNumber(), + ); + expect(Number(traderTokenAccountA.value.amount)).to.be.lessThan( + values.defaultSupply.sub(values.depositAmountA).add(input).toNumber(), + ); + }); }); From 2ee337a2820b2a26e8d61639bc98bc7818f29e2f Mon Sep 17 00:00:00 2001 From: Aung Nanda Oo Date: Thu, 20 Aug 2026 19:03:09 -0700 Subject: [PATCH 2/3] test(#690): assert the exact B->A swap output per Greptile review The new 'Swap from B to A' test asserted a broad range on the output side instead of the precise amount. Compute the expected output by mirroring the on-chain constant-product formula in BN arithmetic (fee, taxed input, then the same mul/div as the program) and assert equality instead of a bound. --- tokens/token-swap/anchor/tests/swap.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tokens/token-swap/anchor/tests/swap.ts b/tokens/token-swap/anchor/tests/swap.ts index 7a724dc12..98bd34c3c 100644 --- a/tokens/token-swap/anchor/tests/swap.ts +++ b/tokens/token-swap/anchor/tests/swap.ts @@ -95,6 +95,13 @@ describe('Swap', () => { it('Swap from B to A', async () => { const input = new anchor.BN(3 * 10 ** 6); + + // Mirror the on-chain constant-product formula exactly, so this + // asserts the precise output amount rather than a broad range. + const feeAmount = input.mul(new anchor.BN(values.fee)).div(new anchor.BN(10000)); + const taxedInput = input.sub(feeAmount); + const expectedOutput = taxedInput.mul(values.depositAmountA).div(values.depositAmountB.add(taxedInput)); + await program.methods .swapExactTokensForTokens(false, input, new anchor.BN(100)) .accountsPartial({ @@ -117,11 +124,8 @@ describe('Swap', () => { expect(traderTokenAccountB.value.amount).to.equal( values.defaultSupply.sub(values.depositAmountB).sub(input).toString(), ); - expect(Number(traderTokenAccountA.value.amount)).to.be.greaterThan( - values.defaultSupply.sub(values.depositAmountA).toNumber(), - ); - expect(Number(traderTokenAccountA.value.amount)).to.be.lessThan( - values.defaultSupply.sub(values.depositAmountA).add(input).toNumber(), + expect(traderTokenAccountA.value.amount).to.equal( + values.defaultSupply.sub(values.depositAmountA).add(expectedOutput).toString(), ); }); }); From c8b1e883ccf2c973faa56d767a52244864c418e9 Mon Sep 17 00:00:00 2001 From: Aung Nanda Oo Date: Fri, 21 Aug 2026 15:40:05 -0700 Subject: [PATCH 3/3] fix(#690): use checked_ math for the fee calculation per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dev-jodee: "should use checked_ math for better handling of overflow" — fee_amount used raw *//in u128 while the rest of the function (including the output computation a few lines below) uses checked_mul/checked_div chains. Switched both fee_amount and taxed_input to checked arithmetic for consistency with the surrounding code's established style. Co-Authored-By: Claude Sonnet 5 --- .../src/instructions/swap_exact_tokens_for_tokens.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs b/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs index b80747146..5cd27c68b 100644 --- a/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs +++ b/tokens/token-swap/anchor/programs/token-swap/src/instructions/swap_exact_tokens_for_tokens.rs @@ -26,10 +26,10 @@ pub fn swap_exact_tokens_for_tokens( }; // Apply trading fee, used to compute the output. - // u128 avoids overflow when input * fee approaches u64::MAX. + // u128 + checked math avoids overflow when input * fee approaches u64::MAX. let amm = &ctx.accounts.amm; - let fee_amount = ((input as u128) * (amm.fee as u128) / 10000) as u64; - let taxed_input = input - fee_amount; + let fee_amount = (input as u128).checked_mul(amm.fee as u128).unwrap().checked_div(10000).unwrap() as u64; + let taxed_input = input.checked_sub(fee_amount).unwrap(); let pool_a = &ctx.accounts.pool_account_a; let pool_b = &ctx.accounts.pool_account_b;