160 lines
18 KiB
Markdown
160 lines
18 KiB
Markdown
# 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:** ✅
|