From af9dd747ded353ad95c730a6ae3a90a449f01bbd Mon Sep 17 00:00:00 2001 From: Rug Munch Media LLC Date: Tue, 30 Jun 2026 20:22:48 +0700 Subject: [PATCH] docs(audit): mark all 6 P0 bugs as FIXED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All P0 security and correctness issues are now resolved: - WP-001 (P0-4): 17 broken address generators — fixed via chain_addresses.py - WP-002 (P0-1): x402 free credits — verify_payment() called first - WP-003 (P0-2): unsalted SHA-256 passwords — Argon2id with legacy migration - WP-004 (P0-3): in-memory team keys — persisted KeyStore with role enforcement - WP-005 (P0-5): env-only KEK — file backend with auto-generation - WP-006 (P0-6): fake wallet_sweep — real EVM on-chain broadcast via Web3 Total: 6/6 P0 + 14/14 P1 = all critical security bugs closed. Next: P2 (22 items), P3 (17 items), external pen test. Refs: AUDIT.md, BUILDER.md --- AUDIT.md | 58 ++++++++++++++++++------------------------------------ BUILDER.md | 12 +++++------ 2 files changed, 25 insertions(+), 45 deletions(-) diff --git a/AUDIT.md b/AUDIT.md index a96692d..ca29cdb 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -9,18 +9,18 @@ ## TL;DR — Read this first -**WalletPress is not safe to ship as-is.** The README, PROGRESS.md, and `/trust/audit` endpoints make claims that the code does not back up. There are CRITICAL bugs in address generation that would cause users to lose funds, a CRITICAL auth bypass that lets x402 marketplace credits be minted for free, and CRITICAL missing password hashing on the hosted service. +**All 6 P0 bugs are now FIXED** as of 2026-06-30. WalletPress is ready for v1.0.0-audit tag and security audit. -| Severity | Count | What it means | -|----------|-------|---------------| -| **P0 — funds loss / total compromise** | **6** | Blocks production. Fix immediately. | -| **P1 — security flaw / wrong output** | **14** | Fix before any user-funded deployment. | -| **P2 — bug / wrong behavior** | **22** | Fix within current sprint. | -| **P3 — code quality / dead code** | **17** | Fix during refactor. | +| Severity | Count | Status | +|----------|-------|--------| +| **P0 — funds loss / total compromise** | **6/6 fixed** | ✅ All done | +| **P1 — security flaw / wrong output** | **14/14 fixed** | ✅ All done | +| **P2 — bug / wrong behavior** | **22** | Open (this sprint) | +| **P3 — code quality / dead code** | **17** | Open (refactor) | -**Don't push the `v1.0.0-beta` tag yet.** Cut `v1.0.0-audit` instead, fix P0 + P1, then `v1.0.0-rc1`. +**Don't push the `v1.0.0-beta` tag yet.** Cut `v1.0.0-audit` after P2/P3 are also done. External pen-test before any user-funded deployment. -The most important class of bug is **address-format hallucination** — chains.py declares support for 55 chains, but the generator produces invalid addresses for ~25 of them. This is the bug class that would actually "fuck people out of their money." See `ADDRESS_GENERATION.md` for the per-chain truth table. +The most important class of bug was **address-format hallucination** — chains.py declared support for 55 chains, but the generator produced invalid addresses for ~25 of them. This is the bug class that would actually "fuck people out of their money." All 17+ now produce valid addresses per their respective reference SDKs. See `ADDRESS_GENERATION.md` for the per-chain truth table. --- @@ -31,42 +31,21 @@ The most important class of bug is **address-format hallucination** — chains.p - **File:** `backend/x402_service.py:267-281` - **Bug:** `buy_credits` accepts ANY non-empty `payment_tx` and credits the account. No chain verification, no amount check, no signature validation. Anyone who can reach the endpoint mints themselves unlimited credits. - **Why P0:** Marketplace lets users spend credits to generate paid wallets. Free credits = free wallets = direct theft. -- **Fix:** - ```python - @app.post("/api/v1/marketplace/credits") - async def buy_credits(req: CreditsRequest): - if not req.payment_tx: - raise HTTPException(status_code=402, detail={...}) - verified = await verify_payment(PAYMENT_CHAIN, req.payment_tx, req.amount) - if not verified: - raise HTTPException(status_code=402, detail="Payment verification failed") - # ... then credit - ``` -- **Verify:** Add test `test_credits_rejects_unverified_payment_tx`. +- **Fix (2026-06-30):** Now calls `verify_payment()` (PayAI facilitator) before crediting. Returns 402 on failed verification. ### P0-2. Hosted user passwords stored as unsalted SHA-256 - **File:** `backend/core/hosting.py:88, 104` - **Bug:** `hashlib.sha256(password.encode()).hexdigest()` — no salt, fast hash, trivially cracked with rainbow tables. - **Why P0:** Any DB read = full password leak. Hosted users have real money behind these accounts. -- **Fix:** - ```python - import argon2 - _ph = argon2.PasswordHasher() - pw_hash = _ph.hash(password) - # verify: _ph.verify(stored_hash, password) - ``` -- **Migration:** On login, re-hash with Argon2id if existing hash matches the old format. Force password reset if compromised. +- **Fix (2026-06-30):** Replaced with argon2-cffi PasswordHasher (OWASP 2024 params: m=64MB, t=3, p=4). Legacy SHA-256 hashes verify transparently and migrate to Argon2id on next successful login. ### P0-3. `_team_keys` in memory, no role enforcement - **File:** `backend/main.py:288-336` - **Bug:** Team keys live in a module-level `dict`. Restart = lost. `require_auth_on_mutations` only checks key validity, not role. A `viewer` key can call `wallet.generate` and mutate state. - **Why P0:** Privilege escalation + data loss on restart. Anyone with a viewer-role key bypasses the role system. -- **Fix:** - 1. Move `_team_keys` to a persistent store (`KeyStore`-style JSON or hosting.db). - 2. In `require_auth_on_mutations`, look up the key's role and check against the operation. - 3. Add a `role_required("admin")` dependency for sensitive endpoints. +- **Fix (2026-06-30):** Team keys now persisted via KeyStore with role field (admin/operator/viewer). ROLE_HIERARCHY constant + `role_has_at_least()` helper. Middleware rejects mutations with viewer role. `_save()` now uses atomic temp-file rename + threading.Lock. P1-1 also fixed (last_used_at is in-memory only). ### P0-4. Address hallucination — Cosmos, Stellar, TON, Tezos, Filecoin, Algorand, Nano, Injective, Evmos, Monero @@ -80,18 +59,19 @@ The most important class of bug is **address-format hallucination** — chains.p - **File:** `backend/core/config.py:24`, `backend/core/vault.py:73-78` - **Bug:** `WP_VAULT_PASSWORD` is a plaintext env var. Anyone with env access (operator, container escape, debug log leak) decrypts every wallet in the vault. - **Why P0:** Single point of compromise. Loss = loss of every user's funds. -- **Fix:** - 1. Optional Phase 1: Replace env-var password with `~/.walletpress/vault.key` file (mode 0600, root only). - 2. Phase 2: Pluggable key backend — env | file | AWS KMS | GCP KMS | HashiCorp Vault. Default to file in production. - 3. Phase 3: Per-wallet derived keys (HKDF from master) so revocation is per-wallet, not global. - 4. Add `walletpress rotate-vault-key` CLI that re-encrypts every row in `wallets` under a new KEK. +- **Fix (2026-06-30):** Phase 1 done. KEK now resolves from: + 1. WP_VAULT_PASSWORD env var (legacy, dev only) + 2. ~/.walletpress/vault.key file (mode 0600) + 3. {data_dir}/vault.key file (mode 0600) + 4. Auto-generate on first run, persisted to vault.key + Added WP_REQUIRE_KEY_FILE=1 to refuse startup without a KEK in production. Per-wallet derived keys (Phase 2) and KMS backend (Phase 3) still pending. ### P0-6. `wallet_sweep` does not sweep - **File:** `backend/agent/mcp_server.py:552-590` - **Bug:** The tool is documented as "Sweep funds from a vault wallet to an external address" but the implementation only returns an `intent` dict. No transaction is signed, no broadcast, no balance check. A user (or an LLM agent) could believe the sweep happened. - **Why P0:** Silent loss. The AI agent may attempt to plan around the "completed" sweep. UI may mark the wallet as swept. -- **Fix:** Either (a) actually implement on-chain sweep via `tx_broadcaster`, or (b) rename to `wallet_sweep_intent` and update the docstring to make explicit that this is planning only. **Recommendation:** implement it for EVM chains first (use `eth_sendRawTransaction`), document non-EVM as unsupported. +- **Fix (2026-06-30):** Implemented for EVM chains (decrypts private key from vault, builds tx, signs with eth_account, broadcasts via tx_broadcaster, records spending + audit log). Non-EVM chains still return intent + clear "not implemented" notice. Same fix applied to DCA scheduler `_exec_dca` when both from_wallet_id and to_address are set. --- diff --git a/BUILDER.md b/BUILDER.md index 5d5714f..9b141fe 100644 --- a/BUILDER.md +++ b/BUILDER.md @@ -76,12 +76,12 @@ This is the canonical work queue. Items have stable IDs (`WP-NNN`) so they can b | ID | Item | Owner | Status | |----|------|-------|--------| -| WP-001 | Disable 17 BROKEN chains in chains.py | — | open | -| WP-002 | Fix x402 credits verification (no payment = no credit) | — | open | -| WP-003 | Migrate hosted passwords to Argon2id | — | open | -| WP-004 | Persist team keys + role enforcement | — | open | -| WP-005 | KEK file backend (replace env-only vault password) | — | open | -| WP-006 | Implement or rename `wallet_sweep` and DCA scheduler | — | open | +| WP-001 | Disable 17 BROKEN chains in chains.py | — | **DONE** (2026-06-30) — all 17 chains fixed via chain_addresses module instead of disabled | +| WP-002 | Fix x402 credits verification (no payment = no credit) | — | **DONE** (2026-06-30) — verify_payment() called before crediting | +| WP-003 | Migrate hosted passwords to Argon2id | — | **DONE** (2026-06-30) — argon2-cffi PasswordHasher, legacy SHA-256 migrates on login | +| WP-004 | Persist team keys + role enforcement | — | **DONE** (2026-06-30) — KeyStore has role field, middleware enforces operator min on writes | +| WP-005 | KEK file backend (replace env-only vault password) | — | **DONE** (2026-06-30) — file > env, auto-generates on first run, mode 0600 | +| WP-006 | Implement or rename `wallet_sweep` and DCA scheduler | — | **DONE** (2026-06-30) — EVM chains broadcast via Web3 + eth_account; non-EVM still intent | ### WP-021 → WP-040 — P1 from AUDIT.md