From 8038fec35bb055d4dddc82ee933e2f78b1a24941 Mon Sep 17 00:00:00 2001 From: Hillaryhardy <73938620+unixwhisperer@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:48:34 +0300 Subject: [PATCH] fix(sbtc-pool-v3): rescale get-share-price to 1e16 so it resolves fee accrual (ajv.3.4) sbtc-pool-v3 mints 1e8 shares per sat but scaled its built-in price by only 1e8, so get-share-price and get-collateral-snapshot.share-price read 1 at launch and ignored any gain short of +100% (F-11 shape, same as the live sbtc-pool-v2). Both now scale by PRICE-PRECISION (1e16): launch reads 1e8, as the header always documented, and a 1-sat fee on a 44,990-sat pool moves it. Share mint/burn math is untouched. stx-pool-v3 has no share-price read-only, so it is unaffected. --- contracts/flashstack-sbtc-pool-v3.clar | 20 +++-- contracts/test/flashstack-sbtc-pool-v3.clar | 20 +++-- docs/TESTNET_STAGING.md | 8 +- docs/security/CLARITY_AUDIT_V2_POOLS.md | 3 +- docs/security/FINDINGS_REGISTER.md | 2 +- docs/security/THREAT_MODEL.md | 2 +- ...btc-pool-v3-share-price-resolution.test.ts | 83 +++++++++++++++++++ 7 files changed, 119 insertions(+), 19 deletions(-) create mode 100644 tests/sbtc-pool-v3-share-price-resolution.test.ts diff --git a/contracts/flashstack-sbtc-pool-v3.clar b/contracts/flashstack-sbtc-pool-v3.clar index daa7819..9ca7760 100644 --- a/contracts/flashstack-sbtc-pool-v3.clar +++ b/contracts/flashstack-sbtc-pool-v3.clar @@ -29,7 +29,7 @@ ;; => each share worth more sBTC over time ;; ;; Collateral oracle: -;; get-share-price returns sats per share (scaled by SHARE-PRECISION) +;; get-share-price returns sats per share, scaled by PRICE-PRECISION (1e16) ;; Lending protocols can call this directly - no external oracle needed ;; Flash loan manipulation-resistant: reserve invariant guarantees ;; pool balance only ever grows by >= fee per loan @@ -80,6 +80,13 @@ (define-data-var reentrancy-locked bool false) (define-constant SHARE-PRECISION u100000000) ;; 1e8 - matches sBTC sat precision +;; ajv.3.4 (F-11 shape): deposit mints 1e8 shares per sat, so one share is +;; worth ~1e-8 sat. Scaling the oracle by SHARE-PRECISION alone made it read +;; 1 at launch and ignore any gain short of +100%. The oracle scales by +;; 1e16 instead, so launch reads 1e8 and it resolves a change of 1 part in +;; 1e8. Read-only output only; share mint/burn math is unchanged. +;; Overflow: (21e6 BTC = 2.1e15 sats) * 1e16 = 2.1e31, far below u128 max. +(define-constant PRICE-PRECISION u10000000000000000) ;; 1e16 = SHARE-PRECISION * 1e8 ;; F-1 fix (v2): virtual shares + virtual assets (OpenZeppelin ERC-4626 ;; inflation-attack mitigation). A permanent phantom position absorbs @@ -272,9 +279,10 @@ ;; Read-only - including collateral oracle ;; ============================================= -;; Current value of one pool share in sats, scaled by SHARE-PRECISION (1e8). -;; Divide result by 1e8 to get sats per share. -;; At launch: 1e8 (= 1 sat per share) +;; Current value of one pool share in sats, scaled by PRICE-PRECISION (1e16). +;; Divide result by 1e16 to get sats per share (equivalently, by 1e8 to get +;; sats per 1e8 shares, the amount one sat mints at launch). +;; At launch: 1e8 (= 1e-8 sat per share, i.e. 1 sat per 1e8 shares) ;; Over time: increases as flash loan fees accumulate ;; ;; This is the primary function lending protocols should consume. @@ -286,7 +294,7 @@ (current-shares (var-get total-shares)) ) ;; Virtual-offset price is well-defined at zero shares (no special case). - (ok (/ (* (+ pool-balance VIRTUAL-ASSETS) SHARE-PRECISION) (+ current-shares VIRTUAL-SHARES))) + (ok (/ (* (+ pool-balance VIRTUAL-ASSETS) PRICE-PRECISION) (+ current-shares VIRTUAL-SHARES))) ) ) @@ -336,7 +344,7 @@ (current-shares (var-get total-shares)) ) (ok { - share-price: (/ (* (+ pool-balance VIRTUAL-ASSETS) SHARE-PRECISION) (+ current-shares VIRTUAL-SHARES)), + share-price: (/ (* (+ pool-balance VIRTUAL-ASSETS) PRICE-PRECISION) (+ current-shares VIRTUAL-SHARES)), total-shares: current-shares, pool-balance: pool-balance, yield-accrued: (var-get total-fees), diff --git a/contracts/test/flashstack-sbtc-pool-v3.clar b/contracts/test/flashstack-sbtc-pool-v3.clar index 0b615cc..39b0657 100644 --- a/contracts/test/flashstack-sbtc-pool-v3.clar +++ b/contracts/test/flashstack-sbtc-pool-v3.clar @@ -29,7 +29,7 @@ ;; => each share worth more sBTC over time ;; ;; Collateral oracle: -;; get-share-price returns sats per share (scaled by SHARE-PRECISION) +;; get-share-price returns sats per share, scaled by PRICE-PRECISION (1e16) ;; Lending protocols can call this directly - no external oracle needed ;; Flash loan manipulation-resistant: reserve invariant guarantees ;; pool balance only ever grows by >= fee per loan @@ -80,6 +80,13 @@ (define-data-var reentrancy-locked bool false) (define-constant SHARE-PRECISION u100000000) ;; 1e8 - matches sBTC sat precision +;; ajv.3.4 (F-11 shape): deposit mints 1e8 shares per sat, so one share is +;; worth ~1e-8 sat. Scaling the oracle by SHARE-PRECISION alone made it read +;; 1 at launch and ignore any gain short of +100%. The oracle scales by +;; 1e16 instead, so launch reads 1e8 and it resolves a change of 1 part in +;; 1e8. Read-only output only; share mint/burn math is unchanged. +;; Overflow: (21e6 BTC = 2.1e15 sats) * 1e16 = 2.1e31, far below u128 max. +(define-constant PRICE-PRECISION u10000000000000000) ;; 1e16 = SHARE-PRECISION * 1e8 ;; F-1 fix (v2): virtual shares + virtual assets (OpenZeppelin ERC-4626 ;; inflation-attack mitigation). A permanent phantom position absorbs @@ -272,9 +279,10 @@ ;; Read-only - including collateral oracle ;; ============================================= -;; Current value of one pool share in sats, scaled by SHARE-PRECISION (1e8). -;; Divide result by 1e8 to get sats per share. -;; At launch: 1e8 (= 1 sat per share) +;; Current value of one pool share in sats, scaled by PRICE-PRECISION (1e16). +;; Divide result by 1e16 to get sats per share (equivalently, by 1e8 to get +;; sats per 1e8 shares, the amount one sat mints at launch). +;; At launch: 1e8 (= 1e-8 sat per share, i.e. 1 sat per 1e8 shares) ;; Over time: increases as flash loan fees accumulate ;; ;; This is the primary function lending protocols should consume. @@ -286,7 +294,7 @@ (current-shares (var-get total-shares)) ) ;; Virtual-offset price is well-defined at zero shares (no special case). - (ok (/ (* (+ pool-balance VIRTUAL-ASSETS) SHARE-PRECISION) (+ current-shares VIRTUAL-SHARES))) + (ok (/ (* (+ pool-balance VIRTUAL-ASSETS) PRICE-PRECISION) (+ current-shares VIRTUAL-SHARES))) ) ) @@ -336,7 +344,7 @@ (current-shares (var-get total-shares)) ) (ok { - share-price: (/ (* (+ pool-balance VIRTUAL-ASSETS) SHARE-PRECISION) (+ current-shares VIRTUAL-SHARES)), + share-price: (/ (* (+ pool-balance VIRTUAL-ASSETS) PRICE-PRECISION) (+ current-shares VIRTUAL-SHARES)), total-shares: current-shares, pool-balance: pool-balance, yield-accrued: (var-get total-fees), diff --git a/docs/TESTNET_STAGING.md b/docs/TESTNET_STAGING.md index b524e59..043efa8 100644 --- a/docs/TESTNET_STAGING.md +++ b/docs/TESTNET_STAGING.md @@ -770,10 +770,10 @@ mainnet: - Never `clarinet deployments apply --mainnet` from this repo (`deployments/README.md`, D6/ajv.6.6). - The v2 → v3 LP migration path (`Flashstack-ajv.4.4`) has to be decided first. -- Two findings outside this plan apply before mainnet: sbtc-pool-v3's - `get-share-price` has the F-11 scaling gap (`Flashstack-ajv.3.4`, #123). Rescale it - before mainnet, or accept and document it. If it's rescaled, this plan runs against - the rescaled source. +- sbtc-pool-v3's `get-share-price` F-11 scaling gap (`Flashstack-ajv.3.4`, #123) is + fixed: it now scales by `PRICE-PRECISION` (1e16) and reads `1e8` at launch. This plan + runs against the rescaled source, so a staged instance deployed before the fix is + not the audited version. --- diff --git a/docs/security/CLARITY_AUDIT_V2_POOLS.md b/docs/security/CLARITY_AUDIT_V2_POOLS.md index 1d2c158..c45228f 100644 --- a/docs/security/CLARITY_AUDIT_V2_POOLS.md +++ b/docs/security/CLARITY_AUDIT_V2_POOLS.md @@ -123,7 +123,8 @@ on-chain caller would have to be a published contract that names it. No integrat The same F-11 structure (`VIRTUAL-SHARES` equal to `SHARE-PRECISION`) appears in `flashstack-sbtc-pool-v2`'s built-in `get-share-price` (1e8/1e8). Checked 2026-10-15 (`Flashstack-ajv.4.12`): it is affected the same way. It reads `1` live, and the detail is in -the FINDINGS_REGISTER F-11 row. +the FINDINGS_REGISTER F-11 row. The undeployed successor `flashstack-sbtc-pool-v3` is +rescaled to 1e16 (`Flashstack-ajv.3.4`). ## Notes diff --git a/docs/security/FINDINGS_REGISTER.md b/docs/security/FINDINGS_REGISTER.md index 98fe26c..65bc5b5 100644 --- a/docs/security/FINDINGS_REGISTER.md +++ b/docs/security/FINDINGS_REGISTER.md @@ -36,7 +36,7 @@ admin action (approving a receiver), not a privileged one — see its row.** | 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, paused 2026-10-01**); `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 applied 2026-10-01: Matt called `set-paused true` on both live v2 pools ([`0x1328a12…`](https://explorer.hiro.so/txid/0x1328a12396c2b24a2bf5280758906876b67a65e48afbbc2251d1b8a8e0d0750a?chain=mainnet), block 9100065, stx-pool-v2; [`0x502470a…`](https://explorer.hiro.so/txid/0x502470a976424d2231d58b0ef8398195c718f3f088b68b88c2438c0c3cd2eb05?chain=mainnet), block 9100067, sbtc-pool-v2; both `success`), which blocks `flash-loan` entirely (confirmed in both contracts' source) — `deposit`/`withdraw` correctly unaffected. Re-read 2026-10-13: both `get-stats` return `paused: true`, `total-loans: 0`. With no receiver ever approved, pausing was 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. **Fixed in the repository only (2026-10-05, `Flashstack-ajv.4.9`) — not a mainnet remediation for any deployed contract.** The successors `flashstack-stx-pool-v3` / `flashstack-sbtc-pool-v3` turned out to ship without the lock too (PR #84); both now carry pool-v3's pv3-F1 guard as a single `reentrancy-locked` bool shared by `deposit`, `withdraw` and `flash-loan` (withdraw included for parity with pool-v3, Matt's decision), returning `ERR-REENTRANT` (`u411` / `u712`). Proven by `tests/v3-pools-reentrancy-lock.test.ts` against both pools separately: deposit- and withdraw-reentry from the flash-loan callback are rejected with zero state change, and the lock never stays set after a success, a blocked reentry, or an under-repay revert; the lock-dependent tests (4 per pool) fail against the pre-fix source. A nested `flash-loan` on the same pool never reaches the lock: the Clarity VM aborts it as a `CircularReference`, with or without this fix (pinned by the same file). Both successors remain **undeployed**, and the live v2 pools are immutable and unchanged by this; deploying the successors requires the separate approved process (receiver vetting, testnet plan, mainnet sign-off) and the v2 disposition (`Flashstack-ajv.4.4`). 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-10** | **Critical (mitigated: all 13 de-approved, FlashStack-held balances rescued; one third-party contract's balance rests with its owner)** | `flashstack-stx-core` and `flashstack-sbtc-core` (live, both `SP20XD46…`) — thirteen receivers ever approved on either core | **Unrestricted callback + untrusted repayment destination.** The original three (`alex-arb-receiver`, `bitflow-arb-receiver`, `zest-liquidation-receiver` on stx-core) were fixed 2026-10-09 (below). The Security & Contract Lead then re-audited **every** receiver ever approved on **either** core against its live deployed source and current balance (2026-10-09) and found the same flaw in ten more: none assert `contract-caller == core` on `execute-*-flash`, and all repay to the caller-supplied `core` parameter rather than a hardcoded address — confirmed by direct read on every one of them, not assumed from the shared shape. Critically, **removing a receiver's whitelist entry does not protect a balance already inside it**: `execute-*-flash` has no caller gate, so a direct call bypasses the core (and its whitelist check) entirely — a point the original fix's wording glossed over. Also found: `zest-liquidation-receiver` was approved on **both** cores independently; the original fix only removed it from stx-core. | **Mitigated in stages, 2026-10-09/10 through 2026-10-12 — all 13 de-approved; FlashStack-held balances rescued, one third-party contract's balance rests with its owner.** (1) **Rescued FlashStack's own funds first**, per the Security & Contract Lead's recommended order, since removal alone doesn't stop a direct call: `stx-test-receiver` held 820,000 µSTX, `sbtc-test-receiver` held 95 sats — both recovered by calling `execute-stx-flash`/`execute-sbtc-flash` directly with `core` set to the admin wallet, the same path an attacker would use, done first (`scripts/rescue-vulnerable-receiver-funds.mjs`, Group B). First broadcast attempt aborted (`abort_by_post_condition` — the script set `PostConditionMode.Deny` with no post-condition declared for the transfer it was itself making; caught immediately via on-chain status check, not assumed successful, with zero funds at risk since an aborted tx fully reverts — fixed to `Allow` and re-broadcast). Confirmed second attempt: [`0x1414a72…`](https://explorer.hiro.so/txid/0x1414a724af3ac9693eceff44239095882cef125fb26d2ce40d8bdfad7a11efa5?chain=mainnet) (819,591 µSTX), [`0x78df942…`](https://explorer.hiro.so/txid/0x78df942e101da17620b14926b8f68f325395df65aa40ae613062085cc4684aec?chain=mainnet) (94 sats), both `success`, balances independently re-read post-confirmation (409 µSTX / 1 sat dust remains — not worth a further tx). (2) **Removed all 9 FlashStack-controlled receivers from both cores' whitelists** (`scripts/remove-vulnerable-receivers-phase2.mjs`, admin wallet `SPR9PQAN…`, confirmed to be the current admin of **both** cores): `stx-test-receiver`, `bitflow-arb-receiver-v3`, `bitflow-arb-receiver-v4`, `bitflow-arb-receiver-v2`, `alex-arb-receiver-v2`, `hk-stx-real-receiver-v1` (stx-core); `zest-liquidation-receiver`, `velar-sbtc-arb-receiver`, `sbtc-test-receiver` (sbtc-core) — [`0xe8a3e84…`](https://explorer.hiro.so/txid/0xe8a3e8490b18a51214afc0eeb37fa3a0a9ec1cac554b71d107e9319a01aaf003?chain=mainnet), [`0x0503c2c…`](https://explorer.hiro.so/txid/0x0503c2cdce499f8d9ca719e7942f0b5eb09d8bdc3132e03a303e39c5f6417670?chain=mainnet), [`0x9e95fcf…`](https://explorer.hiro.so/txid/0x9e95fcf4135e3301f7087cfb553298c084e373897390adb738e8dbc07fde0038?chain=mainnet), [`0x3ece422…`](https://explorer.hiro.so/txid/0x3ece4226b2bf6d591405cd990c98301955333124ed37abcef2186019106bd254?chain=mainnet), [`0xe5e67a9…`](https://explorer.hiro.so/txid/0xe5e67a91f03bacba23777c687396a1fc59d1134d6bc48557b78ed490147a1a25?chain=mainnet), [`0x87613df…`](https://explorer.hiro.so/txid/0x87613df524d5b909ade622325d4a8c7d4da64079c40680a10f30d2b67a2cb19e?chain=mainnet), [`0x8f26a1f…`](https://explorer.hiro.so/txid/0x8f26a1f4a263bde52d8ca88d669a64807fcb3b7600cbdb4fcd491f4e75461752?chain=mainnet), [`0x5e732fc…`](https://explorer.hiro.so/txid/0x5e732fcb479bae5a29170181b67afbdf6edcbf7d6363ca43d01fdb8f62a302aa?chain=mainnet), [`0xba095c1…`](https://explorer.hiro.so/txid/0xba095c1f9707d76fc3ddbcbcb471542d4740e242feba5b24996a7520b1d2ddf2?chain=mainnet), all `success`. Independently re-verified after confirmation: `is-approved-receiver` now reads `false` (`ok false` on sbtc-core, matching its wrapped return type) for all nine. **Correction, 2026-10-12:** [`0x87613df…`](https://explorer.hiro.so/txid/0x87613df524d5b909ade622325d4a8c7d4da64079c40680a10f30d2b67a2cb19e?chain=mainnet) above targeted `SP20XD46….hk-stx-real-receiver-v1` — no contract exists at that address, so the call was a no-op and `hk-stx-real-receiver-v1` stayed approved. The Security & Contract Lead's re-sweep found the real deployed contract at `SP3NZYZA88ENNF0FCR57KBGPFY5RAXWHXXVSB6FBW.hk-stx-real-receiver-v1` (no caller check, 0 STX balance). Removed for real: [`0xd6fe02d9…`](https://explorer.hiro.so/txid/0xd6fe02d9c325da5816f65d009cc7e163a61c5c6be61c85c319612940b897d741?chain=mainnet), `success`, independently re-verified `is-approved-receiver` now reads `false`. (3) **`bitflow-arb-receiver-v3` (200,000 µSTX) and `bitflow-arb-receiver-v4` (184,863 µSTX) rescued 2026-10-10** once the deployer wallet (`SP20XD46…`, the `CONTRACT-OWNER` `rescue-stx` is gated to — distinct from the rotated core admin `SPR9PQAN…` used in steps 1-2) was located and funded with 1 STX to cover its own gas (it held 0 STX despite its contracts holding funds): [`0x8059d3b…`](https://explorer.hiro.so/txid/0x8059d3b1d5443e86324035b4277b6a8597ca758e791892493b497ac0d4f7ed2e?chain=mainnet), [`0xf8aefb4…`](https://explorer.hiro.so/txid/0xf8aefb4b455c648c509de48d543fe5135cb9892dea70e4e1e25070e9be0104a2?chain=mainnet), both `success`; a first attempt was rejected pre-broadcast with `NotEnoughFunds` (not from the receiver, from the signer's own empty balance) and consumed no nonce, so nothing was lost or retried blindly. `rescue-stx` takes a plain amount with no fee math, so both receivers now independently re-read `0 µSTX` exactly — no dust. Both were already removed from stx-core's whitelist in step 2. (4) **`seyi-flash-receiver-v1` is a third party's own contract** (`SP6XSRT6QYQ3KJ96Q8JG3N0QVDS4BHKSCHREMFMF.seyi-flash-receiver-v1`, not `SP20XD46…`) — rescuing its balance isn't ours to perform, that's the owner's call, and he's been notified directly. De-approving it from stx-core **is** ours as core admin, independent of that, and is now done: [`0xbd6c7fac…`](https://explorer.hiro.so/txid/0xbd6c7faca384a7e880e0df954e8cb60d1e460038cd7f49c0658ad864065b0b9e?chain=mainnet), `success`, independently re-verified `is-approved-receiver` now reads `false`. `alex-arb-receiver-v3`/`v4`/`v5` remain correctly untouched (hardened, re-checked `true`). `arkadiko-liquidation-receiver` isn't deployed. Original three (2026-10-09): [`0xcb21695…`](https://explorer.hiro.so/txid/0xcb216957c0051a62b35fd97a4753e6670b231b912c35cd3804735086eb2cae25?chain=mainnet), [`0x8d9057a…`](https://explorer.hiro.so/txid/0x8d9057a6408d824aaf6f401e1f6eaa99edceb97b7f721d64412cc6d227de31a6?chain=mainnet), [`0x290d652…`](https://explorer.hiro.so/txid/0x290d652878e8b1b450ca6636d6da5c5b365b9668b006fc606977ac23d881f2d9?chain=mainnet). No code fix possible or needed on any of these receivers — they're de-listed/drained, not patched, same posture as F-9's live pools. Found while assessing which already-built ALEX/Bitflow/Velar/Arkadiko/Zest integrations were ready to activate, 2026-10-07; re-audited and expanded by the Security & Contract Lead, 2026-10-09. | -| **F-11** | **Low (latent)** | `flashstack-pool-oracle-v2` (live, `SPR9PQAN…`) | **`get-share-price` has no usable resolution.** One pool share is worth about 1e-6 uSTX, but the price is scaled by only 1e6. It reads `1` at the launch value, ignores any gain short of +100%, and reads `0` for any loss below launch. The live value sits on that edge (balance 1,000,000 uSTX, 1e12 shares, price `1`, read 2026-10-14). `get-collateral-snapshot.share-price` is the same expression. Sharpens F-3. **Also in `flashstack-sbtc-pool-v2` (live, `SPR9PQAN…`, paused), checked 2026-10-15 (`Flashstack-ajv.4.12`):** its built-in `get-share-price` and `get-collateral-snapshot.share-price` mint 1e8 shares per sat and scale by only 1e8. They read `1`, not the `1e8` the header documents. Live: balance 44,990 sats, 4,499,000,000,000 shares, price `1`. It stays `1` up to a balance of 89,980 sats (+100%) and reads `0` mid-flash-loan. The deployed source matches the repo byte for byte. The undeployed successor `flashstack-sbtc-pool-v3` has the same expression. | **Open, documented** (immutable). Integrators must use `get-lp-value`. Pinned by `tests/flashstack-pool-oracle-v2.test.ts` (F-11 tests, mutation-checked: raising the scale turns them red). Detail: `CLARITY_AUDIT_V2_POOLS.md`, oracle-v2 section. Found by the Security & Contract Lead, 2026-10-14 (`Flashstack-ajv.4.2`). sbtc-pool-v2 is pinned by `tests/sbtc-pool-v2-share-price-resolution.test.ts` (mutation-checked: raising its `SHARE-PRECISION` to 1e16 turns all 3 tests red). No on-chain contract and no web code reads sbtc-pool-v2's share price today. The web only reads the pool's stats. Same integrator rule: use `get-lp-value`. The sbtc-pool-v3 oracle should be rescaled before it ships. | +| **F-11** | **Low (latent)** | `flashstack-pool-oracle-v2` (live, `SPR9PQAN…`) | **`get-share-price` has no usable resolution.** One pool share is worth about 1e-6 uSTX, but the price is scaled by only 1e6. It reads `1` at the launch value, ignores any gain short of +100%, and reads `0` for any loss below launch. The live value sits on that edge (balance 1,000,000 uSTX, 1e12 shares, price `1`, read 2026-10-14). `get-collateral-snapshot.share-price` is the same expression. Sharpens F-3. **Also in `flashstack-sbtc-pool-v2` (live, `SPR9PQAN…`, paused), checked 2026-10-15 (`Flashstack-ajv.4.12`):** its built-in `get-share-price` and `get-collateral-snapshot.share-price` mint 1e8 shares per sat and scale by only 1e8. They read `1`, not the `1e8` the header documents. Live: balance 44,990 sats, 4,499,000,000,000 shares, price `1`. It stays `1` up to a balance of 89,980 sats (+100%) and reads `0` mid-flash-loan. The deployed source matches the repo byte for byte. The undeployed successor `flashstack-sbtc-pool-v3` had the same expression. | **Open, documented** (immutable). Integrators must use `get-lp-value`. Pinned by `tests/flashstack-pool-oracle-v2.test.ts` (F-11 tests, mutation-checked: raising the scale turns them red). Detail: `CLARITY_AUDIT_V2_POOLS.md`, oracle-v2 section. Found by the Security & Contract Lead, 2026-10-14 (`Flashstack-ajv.4.2`). sbtc-pool-v2 is pinned by `tests/sbtc-pool-v2-share-price-resolution.test.ts` (mutation-checked: raising its `SHARE-PRECISION` to 1e16 turns all 3 tests red). No on-chain contract and no web code reads sbtc-pool-v2's share price today. The web only reads the pool's stats. Same integrator rule: use `get-lp-value`. **`flashstack-sbtc-pool-v3`: FIXED** (undeployed, `Flashstack-ajv.3.4`): `get-share-price` and `get-collateral-snapshot.share-price` now scale by `PRICE-PRECISION` (1e16), so launch reads `1e8` and a 1-sat fee on a 44,990-sat pool moves it. Share mint/burn math is unchanged. Pinned by `tests/sbtc-pool-v3-share-price-resolution.test.ts` (mutation-checked: putting `SHARE-PRECISION` back into either expression turns it red). | | **F-12** | **Low (latent)** | `flashstack-pool-oracle-v2` | **Values are read live, so they can be read mid-loan.** Inside a `flashstack-stx-pool-v2` flash-loan callback, the oracle reports the LP value minus the loan in flight and a share price of `0`. A lending protocol pricing LP collateral from it could be made to liquidate against a transiently depressed value. Not reachable today: the pool is paused, no receiver has ever been approved, and no consumer is known. | **Open, documented** (immutable). Consumers must never read it inside a flash-loan path. Pinned by the same test file (a mid-loan reader sees price 0 and value = deposit - loan). Detail: `CLARITY_AUDIT_V2_POOLS.md`. Found by the Security & Contract Lead, 2026-10-14 (`Flashstack-ajv.4.2`). | | DEP-1 | Informational (no upstream fix available) | Root `package.json`: `@clarigen/cli` → `chokidar` → `braces`. `web/package.json`: `tailwindcss` → `chokidar`/`micromatch` → `braces`, and `eslint-config-next` → `@next/eslint-plugin-next` → `fast-glob` → `micromatch` → `braces` | `npm audit --audit-level=high` reports a high-severity DoS advisory (`GHSA-vfj7-8cjw-p6xm`, stack-exhaustion via deeply nested brace-expansion patterns) in `braces@3.0.3`, reached through two independent `package.json` trees. Verified 2026-10-04: `3.0.3` is the *latest version `braces` has ever published* — the advisory's range (`<=3.0.3`) covers its entire release history, so there is no patched version to bump to, in either tree. `@clarigen/cli@4.1.7` (root) and `tailwindcss@3.4.19` / `eslint-config-next@16.3.6` (web, both their repo-declared latest-compatible releases) are the consumers; all seven `web/` findings this produces (`braces`, `chokidar`, `micromatch`, `fast-glob`, `tailwindcss`, `eslint-config-next`, `@next/eslint-plugin-next`) collapse to this one root cause, not seven separate issues. | **Open, accepted risk — investigated 2026-10-04, not fixed.** `npm audit fix --force`'s only suggested remediation for the root tree downgrades `@clarigen/cli` from `4.1.7` to `0.2.4` (four major versions back, API-incompatible with `tests/clarigen-setup.ts`'s 4.x usage — would break the whole test suite). For `web/`, its suggested remediation is `tailwindcss` 3→4 (a real but major breaking CSS-engine migration, config-format rewrite) and `eslint-config-next` 16→14 (an actual downgrade, not a fix). Neither applied. The vulnerable code path (`chokidar`'s/`micromatch`'s file-watching and glob-matching) is reachable only through local dev-time tooling — `clarigen generate`, Tailwind's build-time class scanner, ESLint's file globbing — against this repo's own file paths, never external, untrusted, or network-reachable input, and none of it ships into the built app users receive. `Dependency Audit` is a deliberately non-required CI check (`.github/workflows/security.yml`'s own comment explains `continue-on-error` was removed on purpose so findings like this stay visible without blocking merges). Revisit if/when `braces` (upstream, affects both) ships a fix, or if/when the team independently decides to do the Tailwind v4 migration for its own sake. | | DEP-2 | Moderate → Low (partially fixed 2026-10-04) | `web/package.json`: `@stacks/connect` → `@reown/appkit` (WalletConnect/Reown integration) → `@walletconnect/*` → `query-string` → `decode-uri-component`; separately → `bip322-js`/`bitcoinjs-message` → `secp256k1` → `elliptic` | Unlike DEP-1, this chain ships into the built web app (it's the wallet-connect stack real users' browsers load), so it got more scrutiny than "accepted risk" by default. Two independent sub-issues: (1) `decode-uri-component@0.2.2` (moderate, `GHSA-vcc3-ghjq-m6fr`, ReDoS on malformed percent-encoded input) — unlike `braces`, a patched `0.5.0` **does** exist upstream, but `query-string`'s own `package.json` still declares `^0.4.1`, so it never resolves there naturally. (2) `elliptic@6.6.1` (low, `GHSA-848j-6mx2-7j84`, risky crypto primitive) — same unfixable-upstream shape as DEP-1: `6.6.1` is the latest version `elliptic` has ever published, so `bip322-js`/`bitcoinjs-message`/`secp256k1` (used for Bitcoin message-signing in the wallet-connect flow) have nothing newer to move to. `@stacks/connect@8.2.7` (latest available) pins `@reown/appkit` at an exact `1.7.17`, so neither sub-issue is reachable by bumping `@stacks/connect` itself — there is no newer release. | **Sub-issue (1) FIXED** via a `decode-uri-component: "^0.5.0"` entry in `web/package.json`'s existing `overrides` block (same mechanism already used there for `@types/react`/`viem`), forcing the patched version in despite `query-string`'s stale declared range. Verified: `web/npm audit` dropped from 26 vulnerabilities (4 low, 15 moderate, 7 high) to 15 (8 low, 7 high) — every moderate-severity finding in this chain cleared, including all of `@reown/appkit*`/`@walletconnect/*`, which were flagged purely transitively. `decode-uri-component@0.5.0` is a zero-dependency, single-function package (published as a drop-in security patch, not an API rewrite); `npm run build` and the production build's static page generation both succeed post-override. **Sub-issue (2) remains open, accepted risk** — no code change possible; `elliptic` has no patched release to move to, same posture as DEP-1, but tracked separately here because this one *does* reach the shipped app rather than only build tooling. The residual `@reown/appkit`/`@reown/appkit-siwx`/`@reown/appkit-universal-connector`/`@stacks/connect` findings are now `low` (downgraded from `moderate`) because only their transitive dependency on `elliptic` remains flagged. Revisit if `@stacks/connect` ships a release past `8.2.7` with a newer `@reown/appkit` pin, or if `elliptic` ever publishes a fix. **Addendum 2026-10-06:** `npm audit` checks live advisory data, not a pinned snapshot, so re-running it a day later surfaced 3 advisories published since: `source-map-js` (high, `GHSA-68fv-2mgg-jv7q`, event-loop DoS via indexed source-map offsets, via both `next`'s and `tailwindcss`'s own `postcss` dependency) and `postcss-nested`/`postcss-selector-parser` (moderate, same `tailwindcss@3.4.19` chain as DEP-1). `source-map-js` **FIXED** the same way as sub-issue (1) — `1.2.2` exists upstream and is a clean, non-major override (`web/package.json`), build verified. The two PostCSS findings fold into DEP-1's existing Tailwind-v4-migration-gated story, not a new item. Count: 15 → 18 (the 3 new) → 17 (post-fix: 8 low, 2 moderate, 7 high). | diff --git a/docs/security/THREAT_MODEL.md b/docs/security/THREAT_MODEL.md index 013145e..f82fc57 100644 --- a/docs/security/THREAT_MODEL.md +++ b/docs/security/THREAT_MODEL.md @@ -213,7 +213,7 @@ Line numbers refer to `83d08de`. - `flashstack-pool-oracle-v2.test.ts`: "F-11: share-price stays 1 through a +99% value gain and only steps at 2x" - `flashstack-pool-oracle-v2.test.ts`: "F-11/F-12: read inside a flash-loan callback, share-price is 0 and lp-value drops by the loan" - The sbtc-pool-v2 test is in PR #123. -- **Follow-up:** `Flashstack-ajv.3.4`. The undeployed `flashstack-sbtc-pool-v3` has the same expression. +- **Successor:** the undeployed `flashstack-sbtc-pool-v3` had the same expression. It is rescaled to 1e16 (`Flashstack-ajv.3.4`), pinned by `tests/sbtc-pool-v3-share-price-resolution.test.ts`. ## 13. Malicious asset: MITIGATED diff --git a/tests/sbtc-pool-v3-share-price-resolution.test.ts b/tests/sbtc-pool-v3-share-price-resolution.test.ts new file mode 100644 index 0000000..dc9f429 --- /dev/null +++ b/tests/sbtc-pool-v3-share-price-resolution.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it, beforeEach } from "vitest"; +import { Cl } from "@stacks/transactions"; + +/** + * Flashstack-ajv.3.4: flashstack-sbtc-pool-v3 (undeployed) carried the same + * get-share-price expression as sbtc-pool-v2 (F-11, pinned for v2 in + * sbtc-pool-v2-share-price-resolution.test.ts): deposit mints 1e8 shares per + * sat, but the price was scaled by only 1e8, so it read 1 at launch and + * ignored any gain short of +100%. + * + * v3 now scales the price by PRICE-PRECISION (1e16): launch reads 1e8 (the + * value the header documents) and a 1-sat fee on a 44,990-sat pool moves it. + * get-collateral-snapshot.share-price is the same expression and moves with it. + * Only the read-only output changed: the deposit and withdraw amounts below + * are the same as v2's. + * + * Mutation check: putting SHARE-PRECISION back into either price expression + * turns these tests red. + */ + +const POOL = "flashstack-sbtc-pool-v3"; +const SBTC = "sbtc-token"; +const LP_DEP = 44_990; // same deposit as the v2 test, so the numbers compare directly +const LP_SHARES = 4_499_000_000_000n; // 1e8 shares per sat, same as v2 + +describe("flashstack-sbtc-pool-v3: get-share-price resolution (F-11 rescale, ajv.3.4)", () => { + let deployer: string; + let lp: string; + let donor: string; + + const mint = (amount: number, to: string) => + expect(simnet.callPublicFn(SBTC, "mint", [Cl.uint(amount), Cl.principal(to)], deployer).result).toBeOk(Cl.bool(true)); + const price = () => + BigInt((simnet.callReadOnlyFn(POOL, "get-share-price", [], deployer).result as any).value.value); + const snapshotPrice = () => + BigInt((simnet.callReadOnlyFn(POOL, "get-collateral-snapshot", [], deployer).result as any).value.value["share-price"].value); + const lpValue = (who: string) => + Number((simnet.callReadOnlyFn(POOL, "get-lp-value", [Cl.principal(who)], deployer).result as any).value.value); + const donate = (amount: number) => { + mint(amount, donor); + expect( + simnet.callPublicFn(SBTC, "transfer", [Cl.uint(amount), Cl.principal(donor), Cl.principal(`${deployer}.${POOL}`), Cl.none()], donor).result, + ).toBeOk(Cl.bool(true)); + }; + // (balance + 1) * 1e16 / (shares + 1e8), the contract's formula + const expected = (balance: number, shares: bigint) => ((BigInt(balance) + 1n) * 10n ** 16n) / (shares + 10n ** 8n); + + beforeEach(() => { + const a = simnet.getAccounts(); + deployer = a.get("deployer")!; + lp = a.get("wallet_1")!; + donor = a.get("wallet_2")!; + }); + + it("reads 1e8 at launch, as the header documents", () => { + expect(price()).toBe(100_000_000n); + expect(snapshotPrice()).toBe(100_000_000n); + }); + + it("moves with a 1-sat gain on a 44,990-sat pool, where v2's expression stays at 1", () => { + mint(LP_DEP, lp); + expect(simnet.callPublicFn(POOL, "deposit", [Cl.uint(LP_DEP)], lp).result).toBeOk(Cl.uint(LP_SHARES)); + const p0 = price(); + expect(p0).toBe(expected(LP_DEP, LP_SHARES)); + expect(p0).toBe(100_000_000n); + + donate(1); // a single-sat fee + const p1 = price(); + expect(p1).toBe(expected(LP_DEP + 1, LP_SHARES)); + expect(p1).toBeGreaterThan(p0); + expect(snapshotPrice()).toBe(p1); + + donate(LP_DEP); // ~+100%: v2 only now steps 1 -> 2; v3 reads ~2e8 + expect(price()).toBe(expected(2 * LP_DEP + 1, LP_SHARES)); + expect(lpValue(lp)).toBe(89_980); + }); + + it("deposit and withdraw amounts are unchanged by the rescale", () => { + mint(LP_DEP, lp); + expect(simnet.callPublicFn(POOL, "deposit", [Cl.uint(LP_DEP)], lp).result).toBeOk(Cl.uint(LP_SHARES)); + expect(simnet.callPublicFn(POOL, "withdraw", [Cl.uint(LP_SHARES)], lp).result).toBeOk(Cl.uint(LP_DEP)); + }); +});