docs(audit): mark all 6 P0 bugs as FIXED

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
This commit is contained in:
Crypto Rug Munch 2026-06-30 20:22:48 +07:00
parent 8b8e85a24d
commit af9dd747de
No known key found for this signature in database
GPG key ID: 58D4300721937626
2 changed files with 25 additions and 45 deletions

View file

@ -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.
---

View file

@ -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