Files

160 lines
18 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# GPT-5.6 Audit Remediation Progress
**Audit:** [`audits/GPT5.6/audit.md`](audit.md:1)
**Mitigation plan:** [`audits/GPT5.6/audit_mitigation.md`](audit_mitigation.md:1)
**Last updated:** 2026-07-24
## Summary
| Severity | Total | Fixed | In Progress | Not Started |
|---|---:|---:|---:|---:|
| Critical | 2 | 2 | 0 | 0 |
| High | 6 | 6 | 0 | 0 |
| Medium | 8 | 7 | 0 | 0 |
| Low / Info | 4 | 3 | 0 | 0 |
| **Total** | **20** | **18** | **0** | **0** |
## Findings status
| ID | Severity | Finding | Status | Version | Notes |
|---|---|---|---|---|---|
| G56-01 | Critical | Verifier accepts no PQ signatures and missing signatures as successful | ✅ Fixed | v0.0.12 | Strict mandatory algorithm policy, fail-closed on malformed input, structured result fields, 11 adversarial tests |
| G56-02 | Critical | Earliest-valid-anchor rule not implemented; relay discovery selects latest replaceable event | ✅ Fixed | v0.0.14–v0.0.16 | Switched to non-replaceable kind 9999, append-only upgrades with `upgrade_of` tag, `selectCanonicalProofCarrier()` function, removed `limit: 1` from relay queries, updated all docs |
| G56-03 | High | Outer wrapper identity not bound to embedded identity | ✅ Fixed | v0.0.14 | Added `outerPubkey` and `expectedAuthor` options to `verifyNIPQRContent()`, `identityBound` field in result, `validForMigration` requires identity binding, 4 identity binding tests |
| G56-04 | High | Bitcoin verification trusts public API JSON, doesn't validate headers/PoW | ✅ Fixed | v0.0.21 | Tier 1+2: rewrote `fetchBitcoinBlockHeader()` to query ALL providers and cross-check merkle roots (fail on disagreement); added `trustMode` field to `verifyOtsProof()` result (`multi-explorer-checked` / `single-explorer-checked` / `structural-only`); updated verify.html + index.html UI to display trust mode label; removed misleading "lite-client verification" / "independently verifiable" comments. Full light-client PoW verification (Tier 3) remains as future work. 2 test assertions added (trustMode present on verified proof, structural-only on pending) |
| G56-05 | High | Signer output reconstructed without validating returned ID/sig/pubkey/fields | ✅ Fixed | v0.0.20 | Added `validateSignerOutput()` helper: checks all required fields present + typed, verifies no template field (content/tags/created_at/kind) changed, verifies pubkey equals expected identity, recomputes + verifies event ID, verifies Schnorr signature, rejects extra fields. Replaced reconstruction at all 4 `signEvent()` call sites (2 in index.html, 2 in verify.html) with validation + retain-exact-signed-object. 15 new tests (mutated content/tags/kind/created_at, wrong pubkey, wrong ID, invalid sig, extra field, missing field, non-object, malformed id/sig, tag element changed) |
| G56-06 | High | New seed continuity asserted, not proven — no successor signature, no identity match | ✅ Fixed (design decision: assert-only) | v0.0.22 | Design decision: continuity is *asserted* by the old identity's signature, not *proven* by cryptography. No in-browser cryptographic trick can prove the seed and published PQ keys are bound, because a compromised browser controls both the seed display and all signatures. Mode 1 (mnemonic-match) rejected: no current Nostr signers accept seed phrases, so it would require typing a valuable mnemonic into a web page — unacceptable risk. Mode 2 (successor secp signature) rejected: a malicious browser can fake the secp signature just as easily as PQ signatures — adds no real binding. Fix: documented the honest position in README.md + nip_proposal.md (continuity is asserted not proven; only out-of-band verification on a different device can prove the seed matches the published keys); added UI recommendation in index.html seed step to verify out-of-band after migration |
| G56-07 | High | Verifier throws on malformed relay/paste input | ✅ Fixed | v0.0.12, v0.0.19 | v0.0.12 wrapped base64/hex in try/catch + verifyNIPQRContent fail closed + verify.html call-site wrapping. v0.0.19 added structural validation + try/catch to `verifyNostrEvent()` (returns false on malformed input), wrapped `parseOtsFile()` body in try/catch (returns null), added `MAX_OTS_PROOF_SIZE` (1 MiB) and `MAX_EVENT_JSON_SIZE` (64 KiB) size guards, 23 fuzz/property tests (random JSON, random bytes, truncation, oversized inputs, wrong-typed objects) |
| G56-08 | High | Browser origin security insufficient for seed handling | ✅ Fixed | v0.0.23 | Code fixes: extracted all inline JS to external files (`index-app.mjs`, `verify-app.mjs`, `version-display.js`), removed `'unsafe-inline'` from `script-src` in CSP, added `frame-ancestors 'none'` + `<meta name="referrer" content="no-referrer">`, added SRI hashes (`integrity`/`crossorigin`) to nostr-login-lite scripts. Server headers applied to nginx `conf.d/default.conf` (scoped to `/post-quantum/` location only): HSTS, `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `Referrer-Policy: no-referrer`, `Permissions-Policy`, COOP, CORP. Verified headers present on `/post-quantum/` and absent on other paths. Remaining (future): vendor signer scripts into repo for full source auditability |
| G56-09 | Medium | 12-word seed default, warning removed | ✅ Fixed | v0.0.17 | Defaulted `generateSeedPhrase()` and `generateSeedPhraseWithEntropy()` to 24 words (256-bit); added 12/24-word selector UI; restored entropy-strength warning; removed "quantum-safe root of trust" / "Make your Nostr identity quantum-resistant" claims from index.html, README.md, nip_proposal.md; 4 new tests |
| G56-10 | Medium | Secret material not zeroized, retained in globals/DOM/clipboard | ✅ Fixed | v0.0.26 | Sign-out now zeroizes all `Uint8Array` secret buffers (BIP39 seed, secp private key, 5 PQ secret keys) via `fill(0)` before nulling references; clears DOM (seed display, seed input, entropy hint); clears clipboard. Documented the JS zeroization limitation in README (garbage collection doesn't guarantee erasure; real fix is hardware/native signer boundary) |
| G56-11 | Medium | Publication advances despite zero relay acceptance | ✅ Fixed | v0.0.25 | Require at least one relay OK for EACH event (kind 1 + kind 9999) before marking step done; show error + keep button enabled if zero acceptance; show partial-coverage warning if some relays failed |
| G56-12 | Medium | Resume workflow insufficiently bound to original event | ✅ Fixed | v0.0.14, v0.0.25 | v0.0.14: fetch all candidates + match by event ID. v0.0.25: persist complete proof carrier + kind 1 event JSON in `savePendingOts()`; restore complete events on resume (not just ID); validate fetched event signature via `verifyNostrEvent()` before adopting; don't blindly fall back to `allCandidates[0]` — require valid signature + correct pubkey |
| G56-13 | Medium | OTS parser lacks total input/operation budgets, unsafe 32-bit varuint | ✅ Fixed | v0.0.25 | Replaced 32-bit bitwise `readVaruint()` with safe arithmetic (`Math.pow(2, shift)` + max value/length checks); added `OTS_MAX_OPERATIONS` (10000) and `OTS_MAX_BRANCHES` (1000) budgets tracked via a budget object passed through `_parseTimestamp`/`_processTimestampEntry`; `MAX_OTS_PROOF_SIZE` (1 MiB) already added in G56-07 |
| G56-14 | Medium | Dependency resolution not reproducible; lockfile ignored and inconsistent | ✅ Fixed | v0.0.25 | Removed `package-lock.json` from `.gitignore`; generated and committed `package-lock.json`; use `npm ci` for reproducible installs |
| G56-15 | Medium | OpenTimestamps submodule not clonable; historical dependency risk | ✅ Fixed | v0.0.25 | Created `.gitmodules` with correct URL (`https://github.com/opentimestamps/javascript-opentimestamps.git`); `git clone --recursive` now works |
| G56-16 | Medium | Documentation and UI overclaim implementation status | ✅ Fixed | v0.0.14–v0.0.18 | Kind 9999 docs (v0.0.14–v0.0.16); v0.0.17 fixed index.html claims + 24-word default; v0.0.18 reconciled README OTS trust model (API-assisted vs light-client), removed stale "not implemented" items (target-digest binding, test suite, 24-word default), softened nip_proposal.md ("No private key ever touches a server" → NIP-07/remote-signer caveat; pending-proof "establishes submission time" → "evidence of submission"; fixed "republished" → "new proof carrier published"), added research-prototype banners to verify.html + README + nip_proposal + explanation.md + laans_explanation.md + why_what_how.md + all 5 research/ docs |
| G56-17 | Low | Block-height statement not validated | ✅ Fixed | v0.0.27 | Removed unverified `block_height` tag from signed content and "pre-quantum at Bitcoin block height N" from the attestation statement. The OTS proof is the real anchor; the claimed height was redundant and could be wrong (height 0 on API failure) |
| G56-18 | Low | Relay URL policy permits insecure ws:// | ✅ Won't fix | — | Nostr events are public — an attacker reading them over ws:// is not a security concern. The events contain no secrets. wss:// is recommended but not required. |
| G56-19 | Low | Missing event IDs accepted by library verifier | ✅ Fixed | v0.0.27 | `verifyNostrEvent()` now requires `id` to be present and a 64-lowercase-hex string (was optional/truthy-check). 2 new tests: missing id rejected, malformed id rejected |
| G56-20 | Info | Large events may be rejected by relays; interoperability untested | ✅ Fixed | v0.0.27 | UI now shows event size (KiB) in the publish step and warns if >60 KiB (some relays reject large events). Documented relay size limits and future work (compact binary format, target relay policy testing) in README "Not Yet Implemented" table |
## Changes by version
### v0.0.12 — G56-01 fix
- Rewrote `verifyNIPQRContent()` with strict mandatory algorithm policy
- Missing signatures on signature algorithms now fail (not treated as KEM successes)
- Unknown algorithms reported as ignored, not valid evidence
- Duplicate algorithm detection
- Public key length validation
- All base64/verification wrapped in try/catch (fail closed)
- Added `pqProofsValid`, `policySufficient`, `validForMigration`, `errors` fields to result
- Updated `verify.html` to display "PQ algorithm policy" result line
- Added 11 adversarial tests
### v0.0.13 — Checkbox UI fix
- Fixed checkbox fill (background red when checked, not just border)
### v0.0.14 — G56-02 + G56-03 fix
- Changed `NIP_QR_KIND` from 11112 (replaceable) to 9999 (non-replaceable)
- Added `buildProofCarrier()` with `ots_status` and `upgrade_of` tags
- `buildUpgradedEvent()` now creates append-only events (not replacements)
- Added `selectCanonicalProofCarrier()` for earliest-valid-anchor selection
- Removed `limit: 1` from relay queries in verify.html and index.html
- Added identity binding (`outerPubkey`, `expectedAuthor`) to `verifyNIPQRContent()`
- Added `identityBound` field to verification result
- Updated resume workflow to fetch all candidates
- Added 12 new tests (8 for G56-02, 4 for G56-03)
### v0.0.15 — Documentation update
- Updated nip_proposal.md and README.md for kind 9999
### v0.0.16 — Eliminate old references
- Eliminated all remaining "kind 11112" references from user-facing files and docs
- Updated explanation.md, laans_explanation.md, why_what_how.md
- Cleaned up button IDs and import statements in index.html
### v0.0.17 — G56-09 fix
- `generateSeedPhrase()` and `generateSeedPhraseWithEntropy()` now default to 24 words (256-bit entropy)
- Both accept an `entropyBits` argument (128 or 256) and validate it
- Added 12/24-word radio selector to the seed-generation panel in `index.html`; toggling regenerates the seed
- Restored entropy-strength warning (12 words = ~64-bit under Grover, testing only; 24 words recommended)
- Removed inaccurate claims: "Make your Nostr identity quantum-resistant" → "Create an experimental pre-quantum PQ key commitment"; "quantum-safe root of trust" → "recovery root"; "quantum-safe seed phrase" → "seed phrase"
- Added research-prototype warning to the unauthenticated landing card
- Updated README.md entropy table and nip_proposal.md mermaid diagram
- Added 4 new tests (24-word default, 12-word option, invalid entropyBits rejection for both functions)
### v0.0.18 — G56-16 fix (status reconciliation)
- README.md: rewrote OTS verification status section to distinguish API-assisted verification (current, trusts explorer) from full light-client verification (not implemented); added research-prototype banner
- README.md: removed stale "not implemented" items (OTS target-digest binding, test suite, 24-word default); added accurate not-implemented items (light-client verification, multi-explorer cross-check, PQ event auth/rotation, independent implementation)
- nip_proposal.md: softened "No private key ever touches a server" → NIP-07/remote-signer caveat + API-assisted OTS trust model; pending-proof "establishes submission time" → "evidence of submission, not an independent timestamp"; fixed "republished" → "new proof carrier published with upgrade_of tag"
- verify.html: added research-prototype note; rewrote card description to list each verification check as a separate result line (PQ signatures individually, policy sufficiency, identity binding)
- Added research-prototype / design-only banners to explanation.md, laans_explanation.md, why_what_how.md, and all 5 research/ docs
- No new tests (documentation-only change)
### v0.0.19 — G56-07 fix (fuzz testing + fail-closed verifiers)
- `verifyNostrEvent()`: added structural validation (type-checks all required fields) + try/catch; returns `false` on any malformed input instead of throwing
- `parseOtsFile()`: wrapped parser body in try/catch; returns `null` on any malformed/truncated input
- Added `MAX_OTS_PROOF_SIZE` (1 MiB) and `MAX_EVENT_JSON_SIZE` (64 KiB) size guards; oversized inputs rejected before any parsing/decoding work
- `verifyNIPQRContent()`: added content-size guard at entry
- verify.html relay path already had try/catch (G56-01); now defense-in-depth with internal fail-closed
- 23 new fuzz/property tests: random JSON shapes to `verifyNIPQRContent` (1000 iterations), random bytes to `parseOtsFile` (2000 iterations), random objects to `verifyNostrEvent` (500 iterations), truncated valid-prefix bytes, oversized inputs, wrong-typed objects, `isOtsConfirmed` fuzz
### v0.0.20 — G56-05 fix (signer output validation)
- Added `validateSignerOutput(signedEvent, template, expectedPubkey)` helper in `pq-crypto.mjs`:
- Checks all 7 required NIP-01 fields present + correctly typed
- Verifies id (64 hex), pubkey (64 hex), sig (128 hex) format
- Verifies no template field changed (content, tags deep-compare, created_at, kind)
- Verifies returned pubkey equals expected identity
- Recomputes event ID and verifies match
- Verifies Schnorr signature
- Rejects extra unexpected fields
- Returns the validated signed object (not a reconstruction)
- Replaced reconstruction at all 4 `signEvent()` call sites:
- `index.html`: kind 1 announcement (was mixing template + signer fields), proof carrier (same), upgrade (now validated)
- `verify.html`: upgrade (now validated)
- 15 new tests: valid output, mutated content/tags/kind/created_at, wrong pubkey, wrong ID, invalid sig, extra field, missing field, non-object, malformed id/sig, tag element changed, no-expectedPubkey backward compat
### v0.0.21 — G56-04 fix (Bitcoin trust-mode labeling + multi-explorer cross-check)
- Rewrote `fetchBitcoinBlockHeader()` to query ALL providers (blockstream + mempool) and cross-check their merkle roots; fails on disagreement (turns fallback-redundancy into a cross-check)
- Added `trustMode` field to `verifyOtsProof()` return value: `multi-explorer-checked` (both providers agreed), `single-explorer-checked` (only one responded), `structural-only` (no Bitcoin attestation verified)
- Updated verify.html (2 locations) and index.html (1 location) to display the trust mode label alongside verification results
- Removed misleading "lite-client verification" / "independently verifiable against the Bitcoin PoW chain" comments from code
- Full light-client PoW/difficulty/chain-linkage verification (Tier 3) remains as future work
- 2 new test assertions: trustMode present + explorer-checked on verified proof, structural-only on pending proof
### v0.0.22 — G56-06 fix (seed continuity: honest claims + out-of-band verification)
- Design decision: continuity is *asserted* by the old identity's signature, not *proven* by cryptography
- Mode 1 (mnemonic-match) rejected: no current Nostr signers accept seed phrases; would require typing a valuable mnemonic into a web page — unacceptable risk
- Mode 2 (successor secp signature) rejected: a malicious browser can fake the secp signature just as easily as PQ signatures — adds no real binding
- Documented the honest position in README.md (Component 1 section) and nip_proposal.md: continuity is asserted not proven; only out-of-band verification on a different device can prove the seed matches the published keys
- Added UI recommendation in index.html seed step: "Verify out-of-band after migration — take your seed phrase to a different device, derive the keys, confirm the public keys match"
- No new tests (documentation + UI only)
### v0.0.23 — G56-08 fix (browser hardening — code-fixable parts)
- Extracted all inline `<script>` blocks from index.html and verify.html into external files:
- `www/js/version-display.js` (self-initializing version display)
- `www/js/index-app.mjs` (main index.html application module, ~1237 lines)
- `www/js/verify-app.mjs` (main verify.html application module, ~609 lines)
- Removed `'unsafe-inline'` from `script-src` in CSP (both pages); kept for `style-src` (many inline styles remain)
- Added `frame-ancestors 'none'` to CSP (clickjacking protection)
- Added `<meta name="referrer" content="no-referrer">` to both pages
- Added SRI hashes (`integrity="sha384-..."` + `crossorigin="anonymous"`) to nostr-login-lite scripts
- Created `deploy/nginx-security-headers.conf` documenting required server-side headers (HSTS, X-Content-Type-Options, X-Frame-Options, Referrer-Policy, Permissions-Policy, COOP, CORP)
- Remaining: apply nginx config on server; vendor signer scripts into repo for full auditability
- No new tests (HTML/deployment changes)
## Test count
- **Before remediation:** 38 tests
- **After G56-01:** 49 tests
- **After G56-02+03:** 61 tests
- **After G56-09:** 65 tests
- **After G56-16:** 65 tests (docs-only)
- **After G56-07:** 88 tests
- **After G56-05:** 103 tests
- **After G56-04:** 103 tests (2 assertions added to existing tests)
- **After G56-06:** 103 tests (docs + UI only)
- **After G56-08:** 103 tests (HTML/deployment changes)
- **All passing:** ✅