Stacks DEX Pool V5 — AMM Pool Contract

CRITICAL C-01: No Token Pair Binding — Complete Asset Drain

The contract never stores which token contracts were used during initialize-pool. Every function accepts arbitrary <ft-trait> parameters. An attacker can:

1. Deploy a malicious SIP-010 token whose transfer always returns (ok true) but moves nothing
2. Call swap-x-for-y passing the fake token as token-x and the real token as token-y
3. The fake transfer "succeeds" (no tokens sent in), but the contract sends real token-y to the attacker
4. Repeat to drain the entire reserve-y balance

Impact: Total loss of all pool funds. Any user can drain both reserves.

;; Attack: pass fake-token as token-x, real-token as token-y
;; fake-token.transfer returns (ok true) but doesn't move tokens
(contract-call? .pool-v5-c6 swap-x-for-y
  .fake-token    ;; attacker's malicious SIP-010
  .real-token-y  ;; the real token held by pool
  u1000000       ;; "input" amount (fake, costs nothing)
  u1             ;; min-dy
  tx-sender      ;; receive real tokens here
  u999999999     ;; deadline far in future
)

Fix: Store token-x-principal and token-y-principal at initialization. Validate every call:

(define-data-var token-x-addr principal tx-sender)
(define-data-var token-y-addr principal tx-sender)

;; In every function:
(asserts! (is-eq (contract-of token-x) (var-get token-x-addr)) ERR_WRONG_TOKEN)
(asserts! (is-eq (contract-of token-y) (var-get token-y-addr)) ERR_WRONG_TOKEN)

CRITICAL C-02: Bulk Operations Swallow Failures Silently

bulk-swap-x-for-y uses map over swap entries, calling a private wrapper that calls the public swap-x-for-y. In Clarity, map collects all results — if one swap returns (err), the error is captured in the list but other swaps still execute. The outer function returns (ok (list ...)) regardless.

This means: a user submitting 10 bulk swaps might have 3 fail silently. The failed ones revert their individual state changes, but the user sees (ok ...) and may not realize partial execution occurred.

Impact: Users may lose funds through unnoticed partial execution. Combined with C-01, an attacker could mix legitimate and malicious swaps in one bulk operation.

Fix: Use fold with early termination on first error, or validate all inputs before executing any swaps.

HIGH H-01: Fee Recipient Permanently Set by First Caller

initialize-pool sets fee-recipient to tx-sender with no way to change it. If the initializer's key is compromised, all future fees flow to the attacker. There is no admin/governance mechanism to update the fee recipient.

Impact: Permanent fee theft if initializer key is compromised. No recovery mechanism.

Fix: Add a governance mechanism to update fee-recipient with appropriate access controls.

HIGH H-02: No Access Control on Pool Initialization

Anyone can call initialize-pool and become the fee recipient. In a race condition, a front-runner could initialize with dust amounts, becoming the permanent fee recipient while providing negligible liquidity.

Impact: Fee theft via front-running. Initializer with dust amounts captures all future fees.

Fix: Add owner/deployer check: (asserts! (is-eq tx-sender contract-deployer) ERR_UNAUTHORIZED)

MEDIUM M-01: Reserve State Desync Risk

Reserves are tracked via reserve-x and reserve-y state variables rather than querying actual token balances. If tokens are sent directly to the contract (not via swap/add-liquidity), the reserves desync from reality. This creates a permanent arbitrage opportunity or loss depending on direction.

Fix: Add a sync function that reads actual balances, or use balance-based accounting.

MEDIUM M-02: Integer Sqrt Limited Newton Iterations

The int-sqrt function uses exactly 8 Newton iterations from n/2. For extremely large products (token amounts > 10^15 multiplied together), 8 iterations may not converge, producing an incorrect sqrt. This affects initial LP share calculation.

Impact: Incorrect LP shares for initial liquidity deposit with very large token amounts.

Fix: Add a convergence check: (asserts! (<= (* result result) n) ERR_SQRT_FAILED)

MEDIUM M-03: Rounding-Based Value Extraction

Integer division in add-liquidity and remove-liquidity always rounds down. An attacker can repeatedly add/remove small amounts of liquidity to extract rounding dust over many transactions. With enough iterations, this drains value from other LPs.

Fix: Round in favor of the pool (round up for deposits, round down for withdrawals) using (+ (/ (* a b) c) (if (> (mod (* a b) c) u0) u1 u0)).

LOW L-01: No LP Share Transfer Mechanism

LP shares are stored in a map with no transfer function. LPs cannot sell or transfer their position without removing liquidity first. This limits composability — no LP tokens for other DeFi protocols to integrate with.

INFO I-01: Clarity 3 — Missing Clarity 4 Safety Features

Contract uses Clarity 3. Clarity 4 introduced as-contract? with explicit asset allowances, which would make the as-contract calls in swap/remove-liquidity functions safer by restricting which assets the contract can transfer. Upgrading would provide defense-in-depth against C-01.