Files
relicario/docs/superpowers/audits/2026-04-18-initial-security-audit-opus-4-5.md
adlee-was-taken 71d51c0bea docs: add security audits and Plan 4 for blocker fixes
- 2026-04-18 initial audit verification (all fixed except H8)
- 2026-05-01 audit with 8 new findings (B1-B4, I1-I6)
- Plan 4: Security Blocker Fixes implementation plan

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-05-02 00:42:17 -04:00

159 lines
6.8 KiB
Markdown

# Verification: 2026-04-18 Initial Security Audit
**Verified by:** Claude Opus 4.5
**Date:** 2026-05-01
**Methodology:** Code inspection of referenced file paths and line numbers
---
## Summary
| Finding | File Exists | Lines Match Current | Vulnerability Status | Confidence |
|---------|-------------|---------------------|---------------------|------------|
| C1 - Setup web-accessible | ✅ | ❌ refactored | **FIXED** | 10/10 |
| C2 - Message router trusts all | ✅ | ❌ refactored | **FIXED** | 10/10 |
| C3 - Capture innerHTML XSS | ✅ | ❌ refactored | **FIXED** | 10/10 |
| C4 - Autofill no origin check | ✅ | ❌ refactored | **FIXED** | 10/10 |
| H1 - KDF unprefixed concat | ✅ | ❌ refactored | **FIXED** | 10/10 |
| H2 - Master key not zeroized | ✅ | ❌ refactored | **FIXED** | 9/10 |
| H3 - Passphrase gate cosmetic | ✅ | ❌ refactored | **FIXED** | 10/10 |
| H4 - Git shells out unsafely | ✅ | ❌ refactored | **FIXED** | 10/10 |
| H5 - WASM Math.random() | ✅ | ❌ refactored | **FIXED** | 10/10 |
| H6 - Modulo bias | ✅ | ❌ refactored | **FIXED** | 10/10 |
| H7 - rpassword outdated | ✅ | ✅ | **FIXED** | 10/10 |
| H8 - Storage plaintext | ✅ | ⚠️ partial | **ACKNOWLEDGED** | 8/10 |
**Verdict:** All CRITICAL and HIGH findings except H8 have been remediated. The codebase has been significantly refactored since this audit, making line number references obsolete but confirming fixes were applied.
---
## CRITICAL Findings
### C1: Setup wizard web-accessible
- **Original claim:** `web_accessible_resources` with `matches: ["<all_urls>"]` allows any website to inject vault config.
- **Current state:** `extension/manifest.json` line 38 shows `"web_accessible_resources": []` (empty array).
- **Router validation:** `router/index.ts` lines 29-71 verify sender origins (`isPopup`, `isSetup`, `isContent`) and return `unauthorized_sender` for invalid callers.
- **Status:** ✅ **FIXED**
### C2: Service-worker trusts every message
- **Original claim:** `index.ts:116-441` ignores `_sender` and trusts all messages.
- **Current state:** `service-worker/index.ts` is now 100 lines; message handling delegated to modular router.
- **Router checks:**
- `sender.frameId === 0` for content scripts (line 42)
- `sender.id === chrome.runtime.id` (line 43)
- Returns `{ ok: false, error: 'unauthorized_sender' }` for invalid senders
- **Status:** ✅ **FIXED**
### C3: Capture prompt innerHTML injection
- **Original claim:** `capture.ts:172-191` uses innerHTML with attacker-controlled strings in page DOM.
- **Current state:**
- Uses `createShadowHost()` (line 128) for closed Shadow DOM
- Builds DOM via `document.createElement` + `.textContent =` (lines 143-216)
- File header (lines 6-9) documents this pattern
- **Status:** ✅ **FIXED**
### C4: Autofill has no origin check
- **Original claim:** `get_autofill_candidates` accepts URL from message payload; `get_credentials` returns any entry by ID.
- **Current state in `content-callable.ts`:**
- Line 25: `const senderHost = safeHostname(sender.tab?.url ?? '')` — uses sender tab, not message
- Line 44: `if (!itemHost || itemHost !== senderHost) return { ok: false, error: 'origin_mismatch' }`
- Lines 46-51: TOFU origin-ack check before returning credentials
- **Status:** ✅ **FIXED**
---
## HIGH Findings
### H1: Argon2id unprefixed concatenation
- **Original claim:** `crypto.rs:225-227` has `password = passphrase || image_secret` without length prefix.
- **Current state at `crypto.rs:229-236`:**
```rust
let mut password = Zeroizing::new(Vec::with_capacity(8 + nfc_passphrase.len() + 8 + 32));
password.extend_from_slice(&(nfc_passphrase.len() as u64).to_be_bytes());
password.extend_from_slice(&nfc_passphrase);
password.extend_from_slice(&32u64.to_be_bytes());
password.extend_from_slice(image_secret);
```
Also includes NFC normalization (lines 224-227).
- **Status:** ✅ **FIXED**
### H2: Master key never zeroized
- **Original claim:** `Vec<u8>` from `derive_master_key` and intermediates leak into heap.
- **Current state:**
- `crypto.rs:212`: returns `Zeroizing<[u8; 32]>`
- `crypto.rs:232`: password wrapped in `Zeroizing::new()`
- `session.rs` (WASM/CLI): stores keys as `Zeroizing<[u8; 32]>`
- CLI rpassword calls wrapped in `Zeroizing::new()`
- **Note:** JS string zeroization remains a limitation (acknowledged).
- **Status:** ✅ **FIXED**
### H3: Passphrase strength gate cosmetic
- **Original claim:** Extension accepts any non-empty passphrase; CLI only requires 8 chars.
- **Current state:**
- `setup.ts:152,640`: `score < 3` disables button
- `setup.ts:784-789`: server-side re-validation before create
- `generators.rs:124-130`: `validate_passphrase_strength()` requires score >= 3
- **Status:** ✅ **FIXED**
### H4: Git shells out without guards
- **Original claim:** No hooks/gpgsign/editor isolation.
- **Current state in `helpers.rs:41-55`:**
```rust
cmd.args([
"-c", "core.hooksPath=/dev/null",
"-c", "commit.gpgsign=false",
"-c", "core.editor=true",
]);
```
Comment explicitly references "Audit H4".
- **Status:** ✅ **FIXED**
### H5: WASM Math.random()
- **Original claim:** `lib.rs:240-256` uses `Math.random()` for password generation.
- **Current state:** `generate_password` calls `core_generate_password` from relicario-core.
- **generators.rs:**
- Line 6: `use rand::rngs::OsRng;`
- Lines 61-64: Uses `Uniform::from()` with `OsRng`
- No `Math.random()` anywhere in codebase
- **Status:** ✅ **FIXED**
### H6: Modulo bias
- **Original claim:** `main.rs:308-317` uses `% CHARSET.len()`.
- **Current state in `generators.rs`:**
- Line 61: `let dist = Uniform::from(0..charset.len());`
- Line 63: `charset[dist.sample(&mut rng)]` — rejection sampling, no modulo
- **Status:** ✅ **FIXED**
### H7: rpassword 5.0.1 outdated
- **Original claim:** Uses deprecated `prompt_password_stderr`.
- **Current state:** `Cargo.toml` shows `rpassword = "7"`, uses `prompt_password`.
- **Status:** ✅ **FIXED**
### H8: Storage keeps apiToken/imageBase64 plaintext
- **Original claim:** `chrome.storage.local` stores PAT and reference image unencrypted.
- **Current state:** Still true — `popup-only.ts:139-141` stores `vaultConfig` and `imageBase64`.
- **Mitigation:** Acknowledged as design constraint; spec documents that filesystem access to browser profile compromises both factors.
- **Status:** ⚠️ **ACKNOWLEDGED** (not fixed, documented as acceptable tradeoff)
---
## Conclusion
The 2026-04-18 audit identified real vulnerabilities that existed at that time. **All CRITICAL and HIGH findings (C1-C4, H1-H7) have since been remediated** with the exact fixes recommended in the audit. The codebase underwent significant refactoring, making the original line number references obsolete.
H8 remains as an acknowledged design constraint inherent to Chrome extension architecture.
**The audit was accurate and the remediation was thorough.**