d7f9a49fc425
docs: v4.4.6 production security audit report and continuity
- time
- 2026-07-29 21:50 UTC · 2 months ago
- machine
- koh2
- parent
- 57cc863ec76c
↓ peach-browser-d7f9a49f.tar.gzatlas at this save
files changed1 file
AUDIT_4.4.6.md+199 −0AUDIT_4.4.6.md+199 −0
@@ -0,0 +1,199 @@1+# Peach Browser Extension — Security & Reliability Audit2+3+**Version:** 4.4.6 4+**Audit date:** 2026-07-30 5+**Scope:** Full source audit (read-only), delta from v4.0.0 6+**Auditor:** Codex agent (GPT-5.6) 7+**Prior audit:** v4.0.0 8+9+---10+11+## Executive Summary12+13+Peach v4.4.6 is **fit for production**. No critical or high-severity vulnerabilities were identified. The codebase demonstrates mature security engineering: a Rust-compiled Noise protocol core, per-entry authenticated encryption with key separation, constant-time comparisons at every authenticated site, Ed25519-signed device manifests with freshness windows, and a well-bounded sync receive path. The changes since v4.0.0 (recovery hardening, pairing/adoption, SAS verification, decentralized mesh convergence) are implemented safely and follow the established security patterns.14+15+---16+17+## Findings by Threat Level18+19+### CRITICAL20+21+None identified.22+23+### HIGH24+25+None identified.26+27+### MEDIUM28+29+#### M-1: Session key persisted in `chrome.storage.session` (accepted risk)30+31+- **Location:** `src/background/vault.ts:1390–1410`32+- **Detail:** The AES-256 derived key is stored hex-encoded in `chrome.storage.session` to survive MV3 service-worker restarts. This storage is memory-backed and cleared on browser close, but a same-session XSS in an extension page (not a web page) could theoretically read it.33+- **Mitigation in place:** Extension pages have a strict CSP (`script-src 'self' 'wasm-unsafe-eval'; object-src 'none'`), no `eval`, no remote scripts. The content script (which runs in page context) cannot access `chrome.storage.session`. Destructive operations are gated to extension-origin senders.34+- **Verdict:** Accepted MV3 tradeoff. The alternative (re-prompt on every SW restart) is a worse UX with no meaningful security gain given the CSP posture.35+36+#### M-2: Tombstone accumulation without bound37+38+- **Location:** `src/background/sync-vault-adapter.ts:115–130`39+- **Detail:** Every deleted entry leaves a `{id, deletedAt}` tombstone that persists indefinitely and is sent in every sync session. A vault with tens of thousands of deletions over years will grow its sync manifest linearly.40+- **Mitigation in place:** `SYNC_LIMITS.MAX_TOMBSTONE_COUNT` (100,000) caps a single sync payload. Trash-retention purge eventually splices the entry array. Records are tiny (id + timestamp).41+- **Verdict:** Reliability concern for extreme long-term use, not a security issue. Documented in-code as a known limitation with a future pruning path.42+43+#### M-3: Recovery words encode 128 bits of a 256-bit root key44+45+- **Location:** `src/shared/recovery-words.ts:25–30`, `src/background/vault.ts:300–310`46+- **Detail:** The 12-word BIP39 phrase encodes only `rootKey[0..16]`. The recovery KEK is derived from a prefix of the plaintext it wraps. Effective recovery security is 128-bit, not 256-bit.47+- **Mitigation in place:** Documented extensively in-code (PCH-01-005). 128-bit security is the standard for BIP39 12-word phrases and is computationally infeasible to brute-force. The GCM ciphertext leaks nothing without the words.48+- **Verdict:** Intentional design, well-documented, standard security level for mnemonic recovery.49+50+### LOW51+52+#### L-1: `console.log`/`console.warn` statements in production code (76 instances)53+54+- **Location:** Various (`src/background/`, `src/shared/`)55+- **Detail:** 76 console statements remain in non-test source. None log secrets (keys, passwords, entry content) — verified by inspection. They log sync stage names, error messages, and diagnostic counters.56+- **Verdict:** No information leakage risk. Cosmetic/hygiene issue only.57+58+#### L-2: No lint script configured59+60+- **Location:** `package.json`61+- **Detail:** The project has no `lint` script. Type checking (`tsc --noEmit`) and the full test suite (3,638 tests) pass, but there is no static analysis for common JS/TS pitfalls (unused vars, implicit any, etc.).62+- **Verdict:** Reliability hygiene gap, not a security issue. The strict TypeScript config and comprehensive test suite compensate substantially.63+64+#### L-3: `unlockWithKey` accepts a raw hex key without Argon2id65+66+- **Location:** `src/background/vault.ts:1466`67+- **Detail:** Used by Android biometric unlock where the key was derived natively. The browser extension never calls this path from user input — it is only reachable via the Capacitor/native bridge.68+- **Verdict:** Correct separation of concerns. The native side owns the KDF for biometric unlock.69+70+#### L-4: HIBP breach check uses SHA-1 (dictated by API)71+72+- **Location:** `src/background/vault.ts:230–250`73+- **Detail:** The Have I Been Pwned k-anonymity API requires SHA-1 prefixes. This is not a Peach design choice. Response padding is requested (`Add-Padding: true`).74+- **Verdict:** No action possible; the API dictates the hash. The k-anonymity model (5-char prefix only) means the full password hash never leaves the device.75+76+### INFORMATIONAL77+78+#### I-1: Duress slot size ceiling (256 KiB)79+80+- **Location:** `src/background/vault.ts:70–75`81+- **Detail:** Both slots are padded to a fixed 256 KiB ceiling when duress is configured. A vault exceeding this would re-expose the duress bit via ciphertext length divergence. Documented as an extreme edge case.82+83+#### I-2: Subnet probe retired84+85+- **Location:** `src/background/sync-runner.ts:1–15`86+- **Detail:** LAN peer-to-peer sync is retired (`docs/sync-lan-retirement.md`). The subnet probe code remains in `sync-transport.ts` but is unreachable from the production runner. The relay is the sole transport.87+88+#### I-3: E2E passkey bypass compile-time flag89+90+- **Location:** `src/background/messages.ts:95`, `src/background/passkey.ts:25`91+- **Detail:** `__PEACH_E2E_PASSKEYS__` is folded to `false` by Vite's `define` in every production build. Verified in the bundled output by the build script. The gate is fully intact for users.92+93+---94+95+## Security Architecture Assessment96+97+### Cryptography ✅98+99+| Component | Implementation | Assessment |100+|-----------|---------------|------------|101+| KDF | Argon2id (m=64MiB, t=3, p=4) | Exceeds OWASP password-manager tier |102+| Vault encryption | AES-256-GCM, random 96-bit nonce, 128-bit tag, AAD domain separation | Correct |103+| Per-entry sync encryption | HKDF-SHA256 per entry ID, AES-GCM with entry-bound AAD | Prevents slot-swap attacks |104+| Key zeroing | `zeroKey()` on all ephemeral/derived keys after use | Consistent |105+| Self-test | PBKDF2+AES-GCM KAT on every unlock | Catches broken WebCrypto |106+| Cross-platform KAT | Argon2id vector coordinated with Android BouncyCastle | Prevents silent param drift |107+108+### Protocol Security ✅109+110+| Component | Implementation | Assessment |111+|-----------|---------------|------------|112+| Initial pairing | Noise_NK_25519_ChaChaPoly_BLAKE2s (Rust `snow` → WASM) | Correct pattern choice |113+| Reconnect | Noise_KK with both static keys, no NK fallback | Prevents downgrade |114+| Post-handshake auth | HMAC-SHA256 bound to handshake hash + DH + domain separation | Prevents session hijack |115+| SAS | 8-digit from handshake hash (64 PRF bits, negligible modulo bias) | Correct |116+| Device manifest | Ed25519 signed, canonical JSON, ±freshness window, version-pinned | Correct |117+| Revocation | Terminal, key-matched (not just id-matched), self-revocation filtered | Prevents re-admission |118+119+### Input Validation ✅120+121+- Sync frame size: 24 MiB hard cap (MV-3)122+- Entry count: 100,000 per payload123+- Entry plaintext: 512 KiB per entry124+- Tombstone count: 100,000 per payload125+- Timestamp clamping: ±5 minutes forward skew (HV-1)126+- Pairing invitation: 128-bit bearer token, base64url validated127+- Adoption frames: strict type guards on every field128+- Relay URLs: `wss:` only, no credentials in URL129+130+### XSS / Injection Surface ✅131+132+- Content script `innerHTML`: all assignments are static SVG strings or typed numbers — no page-derived data reaches markup133+- Note editor: allow-list sanitizer (12 tags, all attributes stripped)134+- Popup: SolidJS (auto-escaping by default), no `dangerouslySetInnerHTML`135+- CSP: `script-src 'self' 'wasm-unsafe-eval'; object-src 'none'`136+- Message handler: sender validation, destructive ops gated to extension origin137+138+### Reliability ✅139+140+- 164 test files, 3,638 passing tests + 40 todo141+- Typecheck passes (`tsc --noEmit`)142+- Chrome/Firefox/Mobile production builds pass143+- MV3 static-import guard (build fails on runtime `import()` in worker graph)144+- Clipboard clear: eviction-proof (chrome.alarms + offscreen document + retry)145+- Auto-lock: alarm-based + idle detection, recovery-confirm exemption146+- Sync timeout: 30s per session, stage-level diagnostics147+- Mesh listener: bounded retry with exponential backoff, abort on lock148+149+---150+151+## Delta Assessment (v4.0.0 → v4.4.6)152+153+| Change Area | Versions | Safety Verdict |154+|-------------|----------|----------------|155+| Recovery flow hardening (try/finally, canvas ref fix) | 4.0.x | ✅ Safe |156+| Onboarding reveal toggle + copy | 4.0.x | ✅ Safe |157+| Playwright smoke tests (recovery, popup lifecycle) | 4.0.x | ✅ Safe |158+| Pairing/adoption visual QA (QR, SAS, success states) | 4.4.4–4.4.5 | ✅ Safe (CSS/layout only) |159+| CSS cascade fix (compact selector specificity) | 4.4.5 | ✅ Safe |160+| Decentralized mesh convergence (deviceManifestVersion: 3) | 4.4.6 | ✅ Safe |161+| Vouched trust source promotion (direct/vouched → equal authority) | 4.4.6 | ✅ Safe |162+| SYNC_DEVICE_MANIFEST on equal entry versions | 4.4.6 | ✅ Safe |163+164+The mesh convergence change (v4.4.6) is the highest-risk delta. It was implemented correctly:165+- Capability-gated: peers that omit `deviceManifestVersion` retain the legacy fast path166+- Authority model: any admitted, Noise-authenticated peer with a valid signing key may propagate; unknown/revoked keys remain blocked167+- Revocation ordering: absorbed before vouching in the same manifest to prevent race168+- Self-revocation filtered: a peer cannot revoke the receiver's own device169+170+---171+172+## Production Worthiness173+174+**Verdict: APPROVED FOR PRODUCTION**175+176+The codebase demonstrates:177+- Defense-in-depth at every trust boundary (transport + envelope encryption)178+- Constant-time comparisons at all authenticated sites179+- Strict input validation with bounded resource consumption180+- Comprehensive test coverage (3,638 tests) with adversarial and KAT suites181+- Clean separation between the content-script attack surface and the privileged background182+- Documented security model with inline rationale at every non-obvious decision183+- No hardcoded secrets, no eval, no remote code loading184+- MV3 compliance with static-import enforcement185+186+No blocking issues. The MEDIUM findings are accepted design tradeoffs with appropriate mitigations. The LOW findings are hygiene items that do not affect security posture.187+188+---189+190+## Recommendations (non-blocking)191+192+1. Add a lint script (ESLint or Biome) to CI for ongoing hygiene.193+2. Consider pruning tombstones older than N months once all peers confirm receipt.194+3. Reduce console statements to structured debug logging behind a compile-time flag.195+4. Document the duress 256 KiB ceiling in user-facing materials for extreme vault sizes.196+197+---198+199+*End of audit.*