ALEX amm-pool-v2-01

Location: swap-helper-a, swap-helper-b, swap-helper-c

Description: All multi-hop swap functions pass none as the min-dy parameter for intermediate legs. Only the final leg receives the user's slippage check. An attacker can sandwich the intermediate swap to extract value.

(define-public (swap-helper-a
    (token-x-trait <ft-trait>) (token-y-trait <ft-trait>) (token-z-trait <ft-trait>)
    (factor-x uint) (factor-y uint) (dx uint) (min-dz (optional uint)))
  (swap-helper token-y-trait token-z-trait factor-y
    (try! (swap-helper token-x-trait token-y-trait factor-x dx none))  ;; <-- none = no slippage check
    min-dz))

Impact: A sandwich attacker can manipulate the intermediate pool's price between the user's two hops. Even though the final output has a min check, the intermediate pool can be drained of value. For a 3-hop swap (swap-helper-b), two intermediate legs have no protection. For 4-hop (swap-helper-c), three legs are unprotected. This is practically exploitable on Stacks via transaction ordering in the mempool.

Recommendation: Add intermediate slippage checks, or calculate the expected intermediate output off-chain and pass minimum amounts for each leg. Alternatively, make multi-hop swaps atomic with a single slippage check on the final amount relative to the input (which the final min-dy partially achieves, but doesn't protect against intermediate pool manipulation that benefits the attacker at the expense of other LPs).

Note: The final output check does bound total user loss, but LPs in intermediate pools can be harmed by the price manipulation even if the swapper's final output is acceptable.

;; Exploit test for C-01: multi-hop sandwich
(define-public (test-exploit-c01-multihop-sandwich)
  ;; Setup: pools X/Y and Y/Z exist with balanced liquidity
  ;; Attacker front-runs: swap large Y→Z to move Z price up
  ;; Victim: swap-helper-a X→Y→Z — gets fair X→Y, but overpays Y→Z
  ;; Attacker back-runs: swap Z→Y to profit
  ;; Victim's final output may still pass min-dz, but intermediate pool LPs absorbed the loss
  (ok true)
)

Location: set-fee-rate-x, set-fee-rate-y, set-max-in-ratio, set-max-out-ratio, set-start-block, set-end-block, set-threshold-x/y, set-oracle-average, set-oracle-enabled

Description: The authorization check uses OR logic — either the DAO or the pool owner can modify parameters:

(asserts! (or (is-eq tx-sender (get pool-owner pool))
              (is-ok (is-dao-or-extension)))
          ERR-NOT-AUTHORIZED)

This means the pool owner (set at pool creation by anyone) has unilateral control over:

• Fee rates — can set to 100% (ONE_8), making all swaps yield 0 output
• Max in/out ratios — can set to 0, blocking all swaps
• Start/end blocks — can set end-block to 0, disabling the pool
• Oracle parameters — can manipulate oracle-resilient pricing for dependent protocols
• Thresholds — can manipulate small-trade pricing linearization

Impact: A malicious pool owner can front-run swaps by setting extreme fee rates, or manipulate oracle prices used by lending protocols. No timelock or multi-sig requirement limits this power.

Recommendation: Add parameter bounds enforced at the contract level (e.g., max fee rate ≤ 10%). Add a timelock for parameter changes. Consider removing pool-owner privilege for sensitive parameters (oracle, max ratios).

Location: swap-x-for-y, swap-y-for-x, reduce-position

Description: When updating pool balances after swaps or withdrawals, the contract silently clamps to zero instead of reverting:

balance-y: (if (<= balance-y dy) u0 (- balance-y dy))

If the AMM math somehow produces a dy larger than balance-y, the pool's tracked balance goes to zero while the vault may still hold tokens. This creates a permanent mismatch between tracked and actual balances.

Impact: Rather than reverting on an impossible state (which would alert users), the pool silently becomes insolvent in its accounting. The max-out-ratio check in the read-only functions should prevent this, but the check happens in a different function call that could theoretically be bypassed if the registry is updated between calls. In reduce-position, the same pattern exists for both balance-x and balance-y and total-supply.

Recommendation: Replace silent clamping with asserts! that reverts the transaction. If the balance would go negative, something is critically wrong and the transaction should fail loudly.

Location: swap-x-for-y, swap-y-for-x

Description: The fee calculation uses mul-up (rounds up), and the net amount uses a simple subtraction with a floor of zero:

(fee (mul-up dx (get fee-rate-x pool)))
(dx-net-fees (if (<= dx fee) u0 (- dx fee)))

For small swaps with non-trivial fee rates, the entire input can be consumed by the fee. The swap proceeds with dx-net-fees = 0, the AMM math returns dy = 0, the user's tokens are transferred to the vault, and nothing comes back. The min-dy check (default-to u0) passes because 0 >= 0.

Impact: Users who don't set an explicit min-dy lose their entire input on small swaps. The fee goes to the reserve, and the user gets nothing.

Recommendation: Add (asserts! (> dx-net-fees u0) ERR-INVALID-LIQUIDITY) after fee calculation.

Location: swap-x-for-y, swap-y-for-x

Description: The oracle-resilient price is computed by calling get-oracle-resilient which reads current pool state (pre-swap balances), then stored in the pool-updated merge. But the pool balances haven't been updated yet when the oracle is recalculated:

(pool-updated (merge pool {
    balance-x: (+ balance-x dx-net-fees fee-rebate),
    balance-y: (if (<= balance-y dy) u0 (- balance-y dy)),
    oracle-resilient: (if (get oracle-enabled pool)
        (try! (get-oracle-resilient token-x token-y factor))  ;; reads OLD balances
        u0)
    }))

The oracle-resilient value is an EMA between the old oracle-resilient and the current instant price. Since it reads pre-swap balances, the EMA lags by one swap. This is a design choice (delayed oracle), but it means within a single block, multiple swaps progressively diverge the oracle from reality.

Impact: Protocols relying on get-oracle-resilient for pricing (e.g., lending liquidations) see a price that's always one swap behind. An attacker can make a large swap, then use the stale oracle in the same block.

Recommendation: Document this as an intentional lag, or compute oracle-resilient using the post-swap balances.

Location: get-y-given-x, get-x-given-y

Description: For trades below the threshold, the AMM linearly interpolates the output:

(dy (if (>= dx threshold)
    (get-y-given-x-internal balance-x balance-y factor dx)
    (div-down (mul-down dx (get-y-given-x-internal balance-x balance-y factor threshold)) threshold)))

This linear approximation gives a constant price for all sub-threshold trades, regardless of the actual curve. The price at threshold may differ from the true marginal price for small trades (depending on the curve's convexity). An attacker can split a large trade into many sub-threshold trades if the linearized price is more favorable, or combine small trades into one above-threshold trade if the curve price is better.

Impact: Systematic arbitrage of the pricing discontinuity at the threshold boundary. The pool-owner controls the threshold, which interacts with H-01.

Recommendation: Use the actual curve for all trade sizes, or ensure the linearization is always less favorable than the curve (conservative rounding).

Location: create-pool

Description: Anyone (not blocklisted) can create a pool for any token pair with any pool-owner principal. Combined with H-01, this means an attacker can create a pool for legitimate token pairs and set themselves as owner with full parameter control.

Impact: Phishing risk — users might interact with a malicious pool for a legitimate token pair. The registry likely prevents duplicate pools (same token-x, token-y, factor), but different factors allow parallel pools.

Recommendation: Restrict pool creation to DAO-approved principals, or limit pool-owner privileges for user-created pools.

Location: add-to-position vs reduce-position

Description: add-to-position does NOT check the blocklist — any address can add liquidity. But reduce-position DOES check the blocklist. This means a user can deposit funds, then get blocklisted, and their funds are permanently locked.

;; add-to-position: no blocklist check
(asserts! (not (is-paused)) ERR-PAUSED)

;; reduce-position: has blocklist check
(asserts! (not (is-blocklisted-or-default tx-sender)) ERR-BLOCKLISTED)

Impact: Blocklisted users cannot withdraw their LP position. This could be intentional (sanctions compliance) but is asymmetric and not documented.

Recommendation: Either add blocklist check to add-to-position (prevent deposit), or remove it from reduce-position (allow withdrawal of existing funds).

Location: All public functions using as-contract

Description: The contract uses Clarity version 2's as-contract which grants blanket authority over all assets. In Clarity 4, as-contract? with explicit with-ft/with-stx allowances would limit the blast radius. Since this contract delegates to amm-vault-v2-01 and amm-registry-v2-01 via as-contract, any vulnerability in those contracts could be exploited through this contract's authority.

Recommendation: When upgrading to Clarity 4, migrate to as-contract? with explicit asset allowances.

Location: pow-fixed, ln-priv, exp-pos

Description: The custom math library implements ln/exp/pow via Taylor series with hard-coded pre-computed constants. The MAX_POW_RELATIVE_ERROR is set to 4 (4e-8 relative error), and pow-down/pow-up apply this as a bound. However, the actual error of the Taylor approximation depends on the input range, and the 11-term series for exp and 5-term series for ln may exceed this bound for extreme inputs near the boundaries (MAX_NATURAL_EXPONENT = 69e8).

Recommendation: This is a known trade-off in on-chain math libraries. Consider adding fuzz tests to verify error bounds across the full input range.

Location: (define-data-var paused bool true)

Description: The contract deploys in a paused state. Only the DAO can unpause it via pause(false). This is good practice for staged deployments but worth noting — the contract is non-functional until explicitly activated.

Recommendation: No action needed. Good deployment hygiene.