Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Clarinet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@ path = "contracts/test/flashstack-sbtc-pool-v2.clar"
clarity_version = 3
epoch = "3.0"

[contracts.test-sbtc-pool-v2-receiver-deposit-reentrant]
path = "contracts/test/test-sbtc-pool-v2-receiver-deposit-reentrant.clar"
clarity_version = 3
epoch = "3.0"

# --- BC1 fix: two-step admin transfer successors (not yet deployed) ---
[contracts.flashstack-stx-core-v2]
path = "contracts/test/flashstack-stx-core-v2.clar"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
;; TEST-ONLY receiver, adversarial: instead of repaying the loan with a plain
;; sBTC transfer, it calls back into flashstack-sbtc-pool-v2's own `deposit`
;; with (amount + fee). That satisfies the pool's balance-delta repayment
;; check (the pool's sBTC balance genuinely grows by >= fee) while ALSO
;; minting the caller LP shares -- priced against the pool's balance as
;; depressed by this same loan's outbound transfer, not the pool's balance
;; before the loan.
;;
;; SIP-010 counterpart of test-pool-v2-receiver-deposit-reentrant.clar
;; (ajv.4.8): independently proves the same F-9 vector reaches
;; flashstack-sbtc-pool-v2, since its repayment path is a SIP-010 `transfer`
;; call (which asserts tx-sender == sender) rather than `stx-transfer?`, and
;; the two pools share no reentrancy surface with each other.
;;
;; The deposit call is wrapped in `as-contract` so `tx-sender` inside
;; `deposit` resolves to THIS contract's own principal, not the original
;; attacker. Without it, `deposit`'s internal sbtc-token transfer call would
;; need `sender` == the attacker's own principal to satisfy its
;; `tx-sender == sender` check, meaning the attacker would have to fund the
;; self-deposit from their own wallet rather than from the loan proceeds
;; this contract is already holding -- defeating the fee-sized-capital
;; framing, same reasoning as the STX PoC's as-contract correction.
(impl-trait .sbtc-flash-receiver-trait.sbtc-flash-receiver-trait)

(define-public (execute-sbtc-flash (amount uint) (core principal))
(let (
(fee-bp (unwrap! (contract-call? .flashstack-sbtc-pool-v2 get-fee-basis-points) (err u901)))
(raw-fee (/ (* amount fee-bp) u10000))
(fee (if (> raw-fee u0) raw-fee u1))
)
(unwrap! (as-contract (contract-call? .flashstack-sbtc-pool-v2 deposit (+ amount fee))) (err u902))
(ok true)
)
)
2 changes: 1 addition & 1 deletion docs/security/FINDINGS_REGISTER.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ admin action (approving a receiver), not a privileged one — see its row.**
| pv3-F1 | Medium | `flashstack-pool-v3` | A receiver reentering `deposit()` for the same asset during its own flash-loan callback got its real deposit inflow miscounted as fee revenue | **Fixed** — per-asset reentrancy lock (deliberately not global, to preserve legitimate cross-asset flash-loan flows). Found by an external reviewer, independently reproduced before fixing. |
| pv3-F2 | Medium | `flashstack-pool-v3` | Flat virtual-shares constant, not calibrated per asset decimals | **Fixed** — `share-scale` computed from a live `get-decimals()` call at listing time, not a hardcoded table. Proven directly (sBTC → 1e8, USDCx → 1e6). |
| pv3-F3 | Medium | `flashstack-pool-v3` | `deposit` was not gated by pause (oversight — `flash-loan` was) | **Fixed** — `deposit` now asserts both global and per-asset pause, matching `flash-loan`. |
| **F-9** | **Critical** | `flashstack-stx-pool-v2` (**CONFIRMED, live, un-mitigated**); `flashstack-sbtc-pool-v2` (**same code pattern, not yet independently proven**) | **The same bug class as pv3-F1 (reentrant deposit during a flash-loan callback), on the two pools pv3-F1's own fix was never ported to.** Neither pool has a reentrancy lock. A flash-loan receiver can repay by calling the pool's own `deposit` instead of a plain transfer. That satisfies the balance-delta repayment check (the pool's balance genuinely increases by `amount + fee`) but **also mints the caller LP shares**, priced against the pool's balance as depressed by this same loan's outbound transfer — a materially cheaper share price than any honest depositor gets, which dilutes every existing LP. Shares are credited to whichever principal is `tx-sender` for the nested `deposit` call: the receiver contract, if it wraps that call in `as-contract` so the deposit is funded out of the loan it is already holding — the construction that keeps the attack fee-sized. Without `as-contract`, `deposit`'s transfer is instead sourced from the attacker's own wallet, which only looks cheap in a test harness that gives every account a large default balance; in practice that path requires fronting nearly the entire loan amount and isn't a fee-sized attack at all. | **CONFIRMED by a passing simnet proof** (`tests/stx-pool-v2-deposit-reentrancy.test.ts`, receiver `contracts/test/test-pool-v2-receiver-deposit-reentrant.clar`), against a `contracts/test/flashstack-stx-pool-v2.clar` copy that is a faithful localization of the **live, funds-bearing** mainnet contract (`SPR9PQAN…`, 1.000000 STX, `paused=false` as of the CONTRACT_INVENTORY snapshot). Measured in one transaction, with the attacker fronting only fee-sized capital (50,000 µSTX — the real minimum that covers the 0.05% fee on a 99,000,000 µSTX / 99 STX loan; below that the self-deposit fails for insufficient balance) via the `as-contract` construction: the receiver contract ends up with 9,904,940,194,109,305 of 10,004,940,194,109,305 total shares (effectively all of them) worth 99,049,499 µSTX, a ~1,981x return on the attacker's 50,000 µSTX outlay (~2,001x at the exact minimum of 49,500 µSTX — one µSTX less and the self-deposit fails), with the attacker's own wallet spending nothing further once that buffer is in place; the sole honest LP's 100,000,000 µSTX deposit is left worth 1,000,000 µSTX — a 99% loss, extracted in a single transaction. (*Corrected 2026-10-01 — independently re-derived by Matt Glory, whose own repro used `as-contract` and caught that an earlier version of this PoC, without it, had credited shares to the attacker's EOA and silently drawn the deposit from the attacker's own wallet, overstating the return as ~9,905x on a 10,000 µSTX outlay. Re-verified a second time via a from-scratch hand derivation of the share-mint formula plus a fresh on-chain-equivalent read-only query, cross-checked against each other and against a boundary test (49,499 µSTX fails, 49,500 succeeds): this also caught that the share-count figures themselves (previously ending "…109,304 of …209,304") were off by JS `Number()` precision loss on a value exceeding `Number.MAX_SAFE_INTEGER` — corrected here to the exact on-chain uint values. The 99% LP-loss and 99,049,499 µSTX value figures were unaffected by either correction.*) `flashstack-sbtc-pool-v2` was read directly (not independently simnet-proven) and has an identical `deposit`/`flash-loan` structure with no lock — same vector, same exposure, 44,990 sats live. The cores (`flashstack-stx-core`, `flashstack-sbtc-core`) are **not** exposed: their equivalent of `deposit` (`deposit-reserve`) is `tx-sender == admin`-gated, which a receiver callback cannot satisfy since `tx-sender` is the original caller throughout the call chain, not the receiver contract or the admin. | **Open, but CONFIRMED DORMANT as of 2026-10-01** — independently re-verified against live chain state, not assumed: (1) `contracts/test/flashstack-stx-pool-v2.clar` (what the PoC ran against) diffs byte-for-byte against a fresh fetch of the deployed source at `SPR9PQAN…`, differing only in the trait address literal (the expected localization) — the PoC is against the real live contract, not a drifted copy; (2) both live pools read `paused=false`, `total-loans=0` right now via direct read-only calls; (3) the admin wallet's **complete** transaction history (24/24 txs, not a sample) contains **zero** `add-approved-receiver` calls against either `flashstack-stx-pool-v2` or `flashstack-sbtc-pool-v2` — every `add-approved-receiver` call it has ever made targets `flashstack-stx-core` instead. **No receiver has ever been whitelisted on either v2 pool, so the vector is unreachable today** — not because of any mitigation, but because nothing has opened the door yet. A control run (same harness, `test-pool-receiver-good` repaying by plain transfer) credits the caller zero shares, isolating the effect to the deposit-reentrancy specifically, not a harness artifact. **The real deadline is the first `add-approved-receiver` call on either pool** — nothing in the current review process would catch this before that call, since "repays via deposit" and "repays via transfer" both make `flash-loan` return `(ok true)`. **Zero-code mitigation available today: `set-paused true` on both live v2 pools blocks `flash-loan` entirely (confirmed in both contracts' source) — `deposit`/`withdraw` correctly unaffected — though with no receiver ever approved, pausing is precautionary rather than stopping an in-progress loss.** No fix is possible in place; these are immutable deployed contracts. A real fix mirrors pv3-F1's per-asset(-equivalent) lock and would need a v3-equivalent successor for these two pools, the same posture already adopted for BC1. Found by the Security & Contract Lead, 2026-10-01, while writing the threat model (`Flashstack-ajv.1.5`). Tracked in `Flashstack-ajv.4.8`. |
| **F-9** | **Critical** | `flashstack-stx-pool-v2` (**CONFIRMED, live, un-mitigated**); `flashstack-sbtc-pool-v2` (**CONFIRMED by a passing simnet proof, 2026-10-04**) | **The same bug class as pv3-F1 (reentrant deposit during a flash-loan callback), on the two pools pv3-F1's own fix was never ported to.** Neither pool has a reentrancy lock. A flash-loan receiver can repay by calling the pool's own `deposit` instead of a plain transfer. That satisfies the balance-delta repayment check (the pool's balance genuinely increases by `amount + fee`) but **also mints the caller LP shares**, priced against the pool's balance as depressed by this same loan's outbound transfer — a materially cheaper share price than any honest depositor gets, which dilutes every existing LP. Shares are credited to whichever principal is `tx-sender` for the nested `deposit` call: the receiver contract, if it wraps that call in `as-contract` so the deposit is funded out of the loan it is already holding — the construction that keeps the attack fee-sized. Without `as-contract`, `deposit`'s transfer is instead sourced from the attacker's own wallet, which only looks cheap in a test harness that gives every account a large default balance; in practice that path requires fronting nearly the entire loan amount and isn't a fee-sized attack at all. | **CONFIRMED by a passing simnet proof** (`tests/stx-pool-v2-deposit-reentrancy.test.ts`, receiver `contracts/test/test-pool-v2-receiver-deposit-reentrant.clar`), against a `contracts/test/flashstack-stx-pool-v2.clar` copy that is a faithful localization of the **live, funds-bearing** mainnet contract (`SPR9PQAN…`, 1.000000 STX, `paused=false` as of the CONTRACT_INVENTORY snapshot). Measured in one transaction, with the attacker fronting only fee-sized capital (50,000 µSTX — the real minimum that covers the 0.05% fee on a 99,000,000 µSTX / 99 STX loan; below that the self-deposit fails for insufficient balance) via the `as-contract` construction: the receiver contract ends up with 9,904,940,194,109,305 of 10,004,940,194,109,305 total shares (effectively all of them) worth 99,049,499 µSTX, a ~1,981x return on the attacker's 50,000 µSTX outlay (~2,001x at the exact minimum of 49,500 µSTX — one µSTX less and the self-deposit fails), with the attacker's own wallet spending nothing further once that buffer is in place; the sole honest LP's 100,000,000 µSTX deposit is left worth 1,000,000 µSTX — a 99% loss, extracted in a single transaction. (*Corrected 2026-10-01 — independently re-derived by Matt Glory, whose own repro used `as-contract` and caught that an earlier version of this PoC, without it, had credited shares to the attacker's EOA and silently drawn the deposit from the attacker's own wallet, overstating the return as ~9,905x on a 10,000 µSTX outlay. Re-verified a second time via a from-scratch hand derivation of the share-mint formula plus a fresh on-chain-equivalent read-only query, cross-checked against each other and against a boundary test (49,499 µSTX fails, 49,500 succeeds): this also caught that the share-count figures themselves (previously ending "…109,304 of …209,304") were off by JS `Number()` precision loss on a value exceeding `Number.MAX_SAFE_INTEGER` — corrected here to the exact on-chain uint values. The 99% LP-loss and 99,049,499 µSTX value figures were unaffected by either correction.*) `flashstack-sbtc-pool-v2` — **CONFIRMED by a passing simnet proof** (`tests/sbtc-pool-v2-deposit-reentrancy.test.ts`, receiver `contracts/test/test-sbtc-pool-v2-receiver-deposit-reentrant.clar`), 2026-10-04: independently reproduced through the pool's SIP-010 repayment path (a `contract-call?` to `sbtc-token`'s `transfer`, which asserts `tx-sender == sender`) rather than assumed to carry over from the STX proof's `stx-transfer?` path. Measured with a 10,000,000-sat (0.1 BTC) honest LP deposit, a 9,900,000-sat loan (99% of reserve, under the pool's own max-single-loan cap), and a 5,000-sat attacker buffer (covers the 4,950-sat fee): the receiver contract ends up holding shares funded entirely out of the loan proceeds — the attacker's own wallet spends nothing beyond the pre-funded buffer — while the honest LP's value drops below their original deposit. A control receiver that repays by plain transfer instead of `deposit` gets exactly zero shares, isolating the effect to the deposit-reentrancy mechanism and ruling out a harness artifact. Same vector, same exposure, 44,990 sats live. The cores (`flashstack-stx-core`, `flashstack-sbtc-core`) are **not** exposed: their equivalent of `deposit` (`deposit-reserve`) is `tx-sender == admin`-gated, which a receiver callback cannot satisfy since `tx-sender` is the original caller throughout the call chain, not the receiver contract or the admin. | **Open, but CONFIRMED DORMANT as of 2026-10-01** — independently re-verified against live chain state, not assumed: (1) `contracts/test/flashstack-stx-pool-v2.clar` (what the PoC ran against) diffs byte-for-byte against a fresh fetch of the deployed source at `SPR9PQAN…`, differing only in the trait address literal (the expected localization) — the PoC is against the real live contract, not a drifted copy; (2) both live pools read `paused=false`, `total-loans=0` right now via direct read-only calls; (3) the admin wallet's **complete** transaction history (24/24 txs, not a sample) contains **zero** `add-approved-receiver` calls against either `flashstack-stx-pool-v2` or `flashstack-sbtc-pool-v2` — every `add-approved-receiver` call it has ever made targets `flashstack-stx-core` instead. **No receiver has ever been whitelisted on either v2 pool, so the vector is unreachable today** — not because of any mitigation, but because nothing has opened the door yet. A control run (same harness, `test-pool-receiver-good` repaying by plain transfer) credits the caller zero shares, isolating the effect to the deposit-reentrancy specifically, not a harness artifact. **The real deadline is the first `add-approved-receiver` call on either pool** — nothing in the current review process would catch this before that call, since "repays via deposit" and "repays via transfer" both make `flash-loan` return `(ok true)`. **Zero-code mitigation available today: `set-paused true` on both live v2 pools blocks `flash-loan` entirely (confirmed in both contracts' source) — `deposit`/`withdraw` correctly unaffected — though with no receiver ever approved, pausing is precautionary rather than stopping an in-progress loss.** No fix is possible in place; these are immutable deployed contracts. A real fix mirrors pv3-F1's per-asset(-equivalent) lock and would need a v3-equivalent successor for these two pools, the same posture already adopted for BC1. Found by the Security & Contract Lead, 2026-10-01, while writing the threat model (`Flashstack-ajv.1.5`). Tracked in `Flashstack-ajv.4.8`. |

---

Expand Down
1 change: 1 addition & 0 deletions tests/mainnet-plan-guard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const KNOWN_TEST_PATHS = [
"contracts/test/test-receiver-bad.clar",
"contracts/test/test-receiver-good.clar",
"contracts/test/test-sbtc-pool-receiver-good.clar",
"contracts/test/test-sbtc-pool-v2-receiver-deposit-reentrant.clar",
"contracts/test/test-sbtc-receiver-bad.clar",
"contracts/test/test-sbtc-receiver-good.clar",
];
Expand Down
Loading
Loading