Skip to content
Open
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
2 changes: 2 additions & 0 deletions docs/security/FINDINGS_REGISTER.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ admin action (approving a receiver), not a privileged one — see its row.**
| 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` (**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`. |
| 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. |

---

Expand Down
98 changes: 4 additions & 94 deletions web/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"@types/react-dom": "19.3.0",
"viem": {
"ws": "^8.21.0"
}
},
"decode-uri-component": "^0.5.0"
}
}
Loading