cp653-659: offline install through all roles (docker images, mcp user, ipfs deferral) + Step-N-of-N wizard + password step + upgrade mirror rotation + wizard net-loss guard

2026-08-05 18:02:22 -07:00

22 KiB

Raw Permalink Blame History

cp138 deep-deep audit — findings ledger

Campaign: Pre-launch deep-deep, 94 tasks across 11 phases. Trigger: Ken's directive: "do a full deep deep on absolutely everything, every file and script, .md/.ts/all svelte-related" + "put on your black hat. FULL security and code audits." Started: 2026-05-25 (cp138 from cp137 baseline). Closed: 2026-05-25. Outcome: 11 findings shipped, 1 standing follow-up, 0 outstanding HIGH/CRITICAL, smoke suite green at 169/0.

Phase A — Hostile chain-op review (all 17 handlers reviewed)

cp138-A-1 SHIPPED (MED) — ADR-0004 amendment overstated frontend price-provider wiring

Where: docs/adr/0004-price-feeds.md, 2026-05-09 amendment + status header date line.

Claim: Amendment said "frontend defaults to fallback prices unless the user explicitly opts into the indexer-relayed feed via Settings" and "a direct-CoinGecko provider also exists and ships [...]; a user who wants freshest prices and is OK with the IP leak can flip to it."

Reality (caught by orphan-file scan):

apps/web/src/lib/prices/providers/composite.ts — exported, not imported anywhere
apps/web/src/lib/prices/providers/coingecko.ts — exported, not imported anywhere
apps/web/src/lib/prices/index.ts:93 setProvider() — exported, never called
Settings page has no price-provider toggle

The frontend uses fallbackProvider only. Indexer-side composite source IS real and live at /v1/price/..., but the frontend doesn't currently consume it.

Fix: Corrected ADR-0004 amendment to accurately describe state (RFC code parked for a future ApiRelayProvider + Settings opt-in); updated status header date line to reflect partial Phase-3 completion. Added cp138-R-1 (future "ship ApiRelayProvider + Settings opt-in") to REVISIT-LIST.

Severity rationale: MED not HIGH because no user-facing security issue (fallback is static + transparent) and no user-facing privacy regression (the privacy-positive default of "frontend doesn't call CoinGecko directly" is trivially upheld when there's no provider switch). The drift is purely doc↔code accuracy.

cp138-A-2 SHIPPED (MED) — feedbackResponse.ts parseInt-on-BIGSERIAL

Where: apps/indexer/src/indexer/handlers/feedbackResponse.ts:95.

Issue: Was passing parseInt(row.id, 10) as a SQL parameter. feedback.id is BIGSERIAL (range 2^63 ≈ 9.2e18) but Number.MAX_SAFE_INTEGER is 2^53 ≈ 9e15. For realistic message volumes parseInt is safe, but the pattern is wrong because pg accepts BIGINT params as strings; the correct fix preserves full 2^63 range with no cost.

Fix: Pass row.id directly (the BIGINT-as-string pg returned). Same problem exists at 10 other call sites (all parseInt(row.id, 10) for display purposes); tracked as cp138-R-1 since the display path goes to JSON which has no native bigint type anyway.

cp138-A-3 SHIPPED (LOW) — stale comment claimed chat_messages.id is SERIAL

Where: apps/indexer/src/api/chatStream.ts:104.

Issue: Comment said "(safe — chat_messages.id is SERIAL, max ~2^31)". Actual schema declares it BIGSERIAL.

Fix: Comment corrected with note that parseInt is safe at practical Morphit scale + cross-reference to cp138-R-1.

cp138-A-4 SHIPPED (LOW) — operatorPaymentMethod forbidden-char + NFC drift

Where: apps/indexer/src/indexer/handlers/operatorPaymentMethod.ts.

Issue: Used a narrower forbidden-char set than order.ts/feedback.ts/profile.ts (just bidi + ZW + paragraph separators, missing C0/C1 control chars and DEL). Also lacked NFC normalization.

Risk surface: Small — operator-only signer = single trusted account. Operator could accidentally smuggle control chars into their own payment-method display. The inconsistency was the real issue.

Fix: Aligned policy. STRIP_CODEPOINTS_RE now covers \u0000-\u001F\u007F-\u009F\u202A-\u202E\u2066-\u2069\u200B-\u200D\u2028\u2029\uFEFF (matches FORBIDDEN_TEXT_CHARS in order.ts). sanitize() now NFC-normalizes first.

cp138-A-5 SHIPPED (LOW) — operatorBlock.sanitizeReason lacked NFC

Where: apps/indexer/src/indexer/handlers/operatorBlock.ts.

Issue: sanitizeReason() stripped C0/C1 control chars and bidi marks but didn't NFC-normalize first. NFD-decomposed input could carry visually-equivalent codepoints that don't match the strip set.

Fix: Added raw.normalize('NFC') before the for-of strip loop; loop now iterates normalized rather than raw.

Phase A clean-handler summary

All 17 chain-op handlers reviewed deeply: dispatcher (integer-typed savepoint keys), order (NFC + forbidden chars + length caps + duplicate detection), chat (block-check before order-permlink bypass; base64 regex anchored + ReDoS-safe at 10k chars <1ms), feedback (three-prong order verification: exists + subject + fee_status='verified'), feedbackResponse (A-2 fix), operatorRegister (comprehensive SSRF defense: RFC1918 + link-local + IMDS + IPv6 + pseudo-TLD), chatRead (strict ISO-8601 + monotonic-advance via atomic UPDATE WHERE + 60s future skew tolerance + 2020-floor), chatIdentity (RFC 7748 small-order X25519 point rejection including bit-255-masking variants), release (triple-AND trust anchor + XMR view-key privacy invariant), featureBid (11 rejection paths + fee verification + anti-sniping 6h floor), orderCancel (atomic UPDATE WHERE constrained to signer's (account, permlink) + state filter prevents replay), orderReplace (mirrors order.ts validation), profile (NFC + FORBIDDEN_DISPLAY_NAME_CHARS + codepoint count + impersonation check), strangerFee (memo-binding + sibling-op transfer + escalating fees 5/2x/cap 640), feeAttest (2-distinct + 1-non-poster sybil defense), operatorBlock (A-5 fix), operatorPaymentMethod (A-4 fix), block (self-block rejection + action whitelist + "no prior block" rejection).

Phase B — HTTP / API surface (no findings)

Relay security headers (apps/relay/src/middleware/security.ts): nosniff + no-referrer + frame-deny + Permissions-Policy + locked-down CSP (default-src 'none'; frame-ancestors 'none'; base-uri 'none'). Body cap enforces Content-Length + explicit Transfer-Encoding:chunked rejection (HTTP 411) for body-bearing methods.
Indexer security headers (apps/indexer/src/api/middleware/security.ts): matches relay posture per ADR-0006 + cross-origin-resource-policy: cross-origin (public-read API) + Cache-Control defaulting.
CORS (apps/relay/src/middleware/cors.ts): exact-match origin allowlist, no wildcards.
Origin enforcement (apps/relay/src/middleware/origin_enforcement.ts): server-side allowlist for fund-spending endpoints (defends curl bypass of browser CORS).
Error message leakage: sampled 4 catch sites; all return {status:'internal'} to client + log err server-side. No PII / stack / host info leaks.

Phase C — Crypto specifics

cp138-C-1 SHIPPED (MED, was M4 from 2026-04-28 audit) — KDF floor was 6000× too generous

Where:

apps/web/src/lib/crypto/keystore.ts:629-630 (passphrase wrap)
apps/web/src/lib/crypto/yubikey/wrap.ts:53-54 (YubiKey HMAC wrap)

Issue: MIN_KDF_OPSLIMIT = 1, MIN_KDF_MEMLIMIT = 1 MB was way below libsodium's INTERACTIVE (ops=2, mem=64 MiB) which the encrypt path actually uses. Latent downgrade-attack surface: a tampered envelope could claim weak Argon2 params (ops=1, mem=1 MB) and bypass ~6000× of the password-strength wall.

Prior history: Identified as finding M4 by the 2026-04-28 batch-I audit and left open for a month. Closed in cp138.

Fix: Raised both floors to match libsodium INTERACTIVE exactly (MIN_KDF_OPSLIMIT = 2, MIN_KDF_MEMLIMIT = 64 * 1024 * 1024). Floor matches encrypt-time default; no envelope written by Morphit can be downgraded. 184 crypto+auth tests pass post-fix.

Phase C clean-area summary

AEAD nonce uniqueness: every nonce comes from sodium.randombytes_buf(NONCEBYTES). Zero fixed/derived nonces. ✓
BIP-39: uses @scure/bip39 (audited Paul Miller library). ✓
secp256k1 signing: uses dblurt's PrivateKey.sign which internally generates canonical RFC6979 signatures (deterministic k). No nonce reuse. ✓
Random source audit: 2 Math.random() uses in production (modal ID, RPC endpoint shuffle). Both non-security. ✓
Forward secrecy posture: docs/CHAT-CRYPTO.md §1-5 explicitly documents the design decision (stateless ECIES — multi-device default; chat-priv compromise implies posting-priv compromise so per-message rotation defends a small slice while leaving everything else exploitable; fast key rotation is the right defense). Not a gap.

Phase D — Database

cp138-D-1 SHIPPED (LOW) — non-deterministic triggered_at

Where: apps/indexer/src/indexer/loyalty.ts (both INSERT sites at line 159 and 242).

Issue: account_loyalty_milestones.triggered_at had DEFAULT NOW() in the schema and was never explicitly written. NOW() is the indexer's wall clock at insert moment, NOT the block's timestamp. For a deterministic chain-replay system, this would diverge across replays. Column currently unread, but a future reader would hit the determinism gap.

Fix: Both INSERT sites now pass blockTime explicitly. 7 loyalty smoke scenarios + 10 loyalty vitest tests still pass.

cp138-D-2 SHIPPED (MED) — push subscription amplification

Where: apps/relay/src/policy/pushSubscriptions.ts upsert().

Issue: No per-account cap. A user could register thousands of (account, endpoint) pairs and create push-notification amplification surface. The push-sender fan-out loop (pushSender.ts:190) awaits one POST per device per inbound message — every chat message they receive would thrash outbound HTTPS calls against arbitrary push services. Concrete attack: hostile user signs up many endpoints, then asks a popular trader for a long conversation; every reply ties up the relay's outbound HTTPS pool.

Fix: Added MAX_SUBSCRIPTIONS_PER_ACCOUNT = 20. Wrapped upsert in this.db.withTx. Added eviction step before INSERT: if the incoming endpoint is NEW (not an ON CONFLICT path) and existing count is at the cap, DELETE the oldest by created_at ASC first. The 3-step transaction (count → evict → upsert) is atomic — no race between parallel upserts can both see the same under-cap snapshot. 244 relay tests pass.

Phase D clean-area summary

Race conditions: 0 SELECT-then-INSERT patterns without ON CONFLICT — all upserts properly atomic.
SQL injection: 18 template-literal interpolations, all SAVEPOINT names with integer-typed keys — safe.
LIKE-pattern injection: 2 ILIKE queries, both use escapeLike() with explicit ESCAPE '\\'.
FK integrity: all FKs have explicit ON DELETE CASCADE or RESTRICT.
Migration linearization: 27 migrations, no gaps.
Dead fields: only candidate was triggered_at (D-1, fixed).
statement_timeout: pool has connectionTimeoutMillis: 5000 but no statement_timeout. Defensible — operator can set via Postgres postgresql.conf or connection-string ?statement_timeout=30000. Documented as REVISIT-LIST follow-up rather than changing pool defaults (which risks breaking long-running queue drains).

cp138-D-3 IDENTIFIED (LOW practical, MED-on-paper) — matrix-bot-sdk transitive vulnerabilities

Where: package-lock.json — transitive deps of matrix-bot-sdk@0.7.1 (used by apps/matrix-bot).

npm audit summary (audit-level=moderate):

2 critical (form-data unsafe-random-boundary GHSA-fjxv-7rqg-78g4; request SSRF GHSA-p8p7-x288-28g6)
14 moderate (qs DoS, tough-cookie prototype pollution, uuid bounds, esbuild dev-server, vite path traversal in optimized deps)
6 low (deduplicated)

Practical exposure assessment:

form-data boundary uses unsafe random — exploitable only against an attacker-controlled multipart upload, which matrix-bot doesn't accept (it only POSTs JSON to a configured Matrix homeserver). Near-zero practical risk.
request SSRF — exploitable only if user-controlled URLs are passed to it. matrix-bot doesn't accept inbound URLs to fetch; it sends outbound to operator-env-configured homeserver. Near-zero practical risk.
qs/tough-cookie/uuid — same trust-boundary reasoning. matrix-bot doesn't parse user-supplied query strings or accept user cookies.
esbuild/vite — dev-only. Production builds use compiled output served by nginx; the dev server is never exposed to the public.

Why this is "LOW practical": matrix-bot is opt-in (per the design-decision comment in apps/matrix-bot/src/main.ts: "if an operator doesn't use Matrix, the systemd unit can be safely enabled (or not) and the bot will exit cleanly without consuming resources"). An operator who doesn't set MORPHIT_MATRIX_BOT_ALERT_MXID never starts the bot, so the SDK isn't even loaded in their running process. For operators who DO opt in, the SDK is talking outbound to their own configured homeserver — there's no inbound user URL surface.

Why this is "MED-on-paper": The CVEs are real, the SDK ships with deprecated/abandoned request@2.88.2, and an automated CVE scanner will report criticals. For an operator running production, that's a quarterly-review item.

Upstream constraint: Upgrading to matrix-bot-sdk@0.8.0 (latest) doesn't fix it — 0.8.0 still depends on request@^2.88.2 and request-promise@^4.2.6. The fix would require either (a) switching to matrix-js-sdk (the official SDK with a bigger surface) or (b) adding npm overrides to force-resolve transitives, which requires testing matrix-bot's actual code paths still function with the overridden versions.

Fix: Documented in REVISIT-LIST as cp138-R-2 standing follow-up. No code change shipped in cp138 because (a) the practical risk is near-zero given matrix-bot's input surface, (b) the proper fix involves either an SDK swap or npm overrides testing that exceeds pre-launch scope, and (c) operators get clear opt-in semantics today.

OPERATIONS.md update committed: Added a new sub-section documenting the matrix-bot opt-in security posture and the known-issue list with practical-exposure annotations, so operators making the matrix-bot enablement decision have full information.

Phase E — Frontend XSS surface (no findings)

All 16 {@html} sites verified trusted:

8 i18n @html sites — Pinned by i18n-html-injection-smoke.ts which walks every callsite, collects keys, scans all 10 locales for dangerous patterns. Allowed tags: strong/em/b/i/br/a/code/span.
3 QR-code sites (LoginQrInitiator, QrPanel, 2fa) — qrcode npm library toString({type:'svg'}) outputs well-formed SVG.
ProtectedTextarea overlay — User input HTML-escaped via local escapeHtml(); m.kind from closed-set type PrivateKeyMatchKind = 'wif' | 'hex_64' | 'mnemonic'.
Head.svelte onion-location — computeOnionLocation validates .onion suffix; quotes escaped via .replace(/"/g, '"').
Head.svelte JSON-LD — JSON.stringify(node).replace(/</g, '\\u003c') prevents </script> injection.
IdentityLabel + account profile avatar SVG — Sanitized via $lib/avatar/index.ts (39 tests covering script-strip, on*-attr strip, javascript:-URL strip, foreignObject strip, etc.).

Other checks clean: 0 external <script>/<link> tags (no SRI surface needed), 0 postMessage sites, 0 direct external fetches (all RPC goes through endpoint rotator).

Phase F — Static code quality

TODO/FIXME/XXX/HACK: 0 instances in entire codebase. ✓
Math.random in security paths: 0. ✓
Loose equality: 7 intentional == null idiom + 3 inside prose. ✓
Secrets in error/log channels: 0. ✓
Unguarded JSON.parse: 3 candidates, all inside functions whose callers properly try/catch. ✓
Silent catch blocks: 20+ found; sampled 3 — all legitimate "validate or fall through" patterns. ✓
setTimeout/setInterval cleanup: 10 candidates with setTimeout only; all short fire-and-forget. The one "setInterval without clearInterval" hit was actually a comment, not code.
Dead-code orphan exports: noisy scan returned 297 candidates, all type-export false positives. Cp138-A-1 was the one real dead-export case.

Phase G — Regex accuracy

Substantively complete:

ReDoS scan: 8 candidate (X+)* patterns, all the same permlink validator /^[a-z0-9]+(?:-[a-z0-9]+)*$/. Tested at 10k adversarial chars: 0.1ms. Safe (V8 linear-time on non-overlapping match alphabets).
22 unanchored production .test() regexes all verified intentional (scheme prefixes, error-message matching, single-char alnum checks).
Base64 ciphertext regex tested at 10k chars: <1ms. Safe.

Phase H — Smokes

cp138-H-1 SHIPPED (LOW) — persona-walkthrough ALERT_COPY sentinel undercount

Where: apps/web/scripts/persona-walkthrough-smoke.ts P121-CP10-4 sentinel.

Issue: Sentinel listed only 14 of 17 host-resource ALERT_COPY events. mount_critical, mount_warn, mount_info had been added to classifier.ts but never added to the sentinel. The sentinel description "(14 entries)" was also stale.

Fix: Updated sentinel name to enumerate all 17 entries + added the 3 missing mustHave strings. Smoke remains 169/0.

Phase H clean-area summary

17 hardcoded counts verified: "16 assets" ✓, "10 locales" ✓, "6 backlog locales" ✓ (it/pl/ru/fa/zh-CN/zh-HK), "≥20 [lang]/ entries" passes with 31 actual.
Part-N citation staleness: oldest are Part 19 audit-doc citations describing WHEN bugs were discovered. Smokes themselves are still defending live invariants.
12 describe.skipIf(!INTEGRATION_ENABLED) sites — gated on Postgres harness availability, not silent debt.
2 genuine skips both documented (cross-tab jsdom env; module-removed audit trail).
Property-based / mutation testing: not adopted. Future work, not a regression.

Phase I — Documentation accuracy

cp138-I-1 SHIPPED (LOW) — no repo-root SECURITY.md

Where: Repo root.

Issue: Forgejo's convention (like GitHub) is to look for SECURITY.md at root, .forgejo/, or docs/. The full security policy lived only at docs/SECURITY.md. While Forgejo does fall through to docs/, the root convention is friendlier to researchers using auto-discovery tools.

Fix: Added 27-line repo-root SECURITY.md pointing to docs/SECURITY.md with the Matrix DM + Forgejo paths + 72h ack + 7d triage commitments.

Phase I clean-area summary

README.md asset enumeration (16 assets) ✓
RUN-A-MORPHIT-NODE.md "Trade-only assets" heading lists 13 + BTC/XMR/BLURT = 16 ✓
OPERATIONS.md grep for drift indicators: all Part-N citations historical, not stale
Broken doc-relative links: 0 (one false positive on (url) inside a markdown example syntax)
METADATA-LEAK-CATALOG.md claim "no IP addresses logged" verified by access_log.ts source
PRIVACY.md: doesn't exist as separate file — METADATA-LEAK-CATALOG.md + SECURITY.md cover the privacy posture (documented design choice)
FAQ entries: 256 total, asset coverage uneven but expected (BTC/XMR/BLURT most-mentioned)

Phase J — Wiring drift

cp138-J-1 SHIPPED (LOW) — XRP address placeholder unwired

Where: apps/web/src/lib/components/AddressShareModal.svelte.

Issue: Placeholder ternary chain covered 14 of 16 assets (BTC, XMR, USDT, USDC, DAI, BCH, LTC, DASH, DOGE, ZEC, ARRR, DCR, SOL, ETH) but XRP was missing. When method === 'xrp', the chain fell through to BLURT's placeholder. The chat.address.address_placeholder_xrp locale key existed in all 10 locales but was unreferenced. Address VALIDATION for XRP already worked (line 292).

Fix: Inserted the XRP branch between ETH and the BLURT fallback.

Phase J clean-area summary

Locale parity: 9 non-English locales achieve perfect parity with en.json (3,095 keys × 10 locales = 30,950 pairs). 0 orphans, 0 missing. ✓
Code-referenced keys missing from en.json: 0 real cases (1 false positive in a docstring example). ✓

Phase K — Failover / external-dep fallback (no findings)

All-endpoints-failed: endpoint rotator throws cleanly; UI catches and shows degraded state. ✓
Unactionable errors: 1 hit (chat.message.failed_label: 'Failed') — actually correct as a per-message status label, not an error popup. ✓

Standing follow-ups (logged in REVISIT-LIST)

cp138-R-1 (post-launch scaling) — bigint id propagation. 11 sites in apps/indexer/src/{api,indexer}/... use parseInt(row.id, 10) to convert BIGSERIAL ids back to JS numbers. Safe up to 2^53 (~9 quadrillion rows) which is far beyond Morphit's realistic scale, but the correct pattern is to keep ids as strings end-to-end (DB → wire → client) since JSON has no native bigint. Long-horizon scaling item, not a pre-launch blocker.
Ship ApiRelayProvider + Settings opt-in for live prices to deliver the user-facing price-staleness UX that ADR-0004 originally promised. Frontend module exists, indexer endpoint exists; what's missing is the apirelay provider implementation + a Settings toggle. Was overstated as shipped in 2026-05-09; corrected in cp138.
Add statement_timeout guidance to OPERATIONS.md so operators know to set Postgres-side statement_timeout for production deployments (current pool defaults don't ship one to avoid breaking long-running queue drains). — SHIPPED 2026-05-25 (post-cp138) at OPERATIONS.md §37.8 sub-item e., with sentinel coverage in scripts/operations-hardening-smoke.ts and the §37.8 one-liner in RUN-A-MORPHIT-NODE.md §11 updated to match.

Out-of-scope answer to "what would a pro firm do?"

See docs/AUDIT-OUTSIDE-SCOPE.md for the answer to Ken's "would a pro firm find anything I haven't?" question — leverage/urgency table for 15 test categories, top 5 (DAST against running instance, cryptographic specialist review, active fuzzing of parsers, threat modeling workshop, supply-chain audit + SBOM), with budget estimates (tier-1 firm $40-120k vs $5-15k contractor rotation vs $5k HackerOne starter).

End of cp138 audit findings.

22 KiB Raw Permalink Blame History Unescape Escape