laantungir/nostr_quantum_preparation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
401c88511e991825a2370e9d61e8945784ba205a
nostr_quantum_preparation/audits/GLM5.2/findings.md
T

13 KiB
Raw Blame History

Detailed Findings (Second Pass)

Companion to audit.md. Each finding includes the exact code location, the issue, the impact, and a recommended fix.

Severity legend: C = Critical, H = High, M = Medium, L = Low/Informational.

Changes from the first pass: F-C1 (missing successor signature) is reclassified as a documented design choice — no longer a finding. F-C4 (docs vs code mismatch) is resolved — no longer a finding. F-C2 and F-C3 (OTS verification) are resolved — implemented real client-side OTS verification. F-M6 (truncated laans_explanation.md) is resolved. F-L8 is new (minor doc leftover).

F-C2 (Critical) — RESOLVED: OpenTimestamps proofs are now cryptographically verified

Location: www/js/pq-crypto.mjs:875 (isOtsConfirmed), www/js/pq-crypto.mjs:840 (upgradeOts), www/verify.html:539, www/index.html:1455.

Issue. The project's central anti-backdating claim (now explicitly stated in the reconciled docs as the primary anti-forgery mechanism) is that the key-link event is anchored to a Bitcoin block. Verification of this anchor is performed in two ways, both inadequate:

  1. isOtsConfirmed(otsBytes) scans the proof for the 8-byte Bitcoin block-header attestation magic 0588960d73d71901 and returns true if found anywhere:

    const bitcoinTag = hexToBytes('0588960d73d71901');
outer: for (let i = 0; i <= otsBytes.length - bitcoinTag.length; i++) {
    for (let j = 0; j < bitcoinTag.length; j++) {
        if (otsBytes[i + j] !== bitcoinTag[j]) continue outer;
    }
    return true;
}
return false;

    This is a substring search, not a proof verification. It does not parse the OTS op stream, check the Merkle path, or check the block header against the Bitcoin chain.

  2. upgradeOts(otsBytes) POSTs the proof to a server and trusts the JSON response's confirmed boolean.

Neither page runs a real OTS verification. The vendored resources/javascript-opentimestamps/ library — which can perform full verification — is present but not imported.

Impact. A malicious kind 11112 event can embed an ots tag containing arbitrary bytes that include the 8-byte magic and be displayed as "OTS: Confirmed (Bitcoin)" (www/verify.html:548). The pre-quantum anchoring is spoofable. Since the reconciled security model explicitly relies on OTS precedence as the mechanism that distinguishes real from forged events after a quantum break, this is the single most important open finding.

Fix. Use the vendored OTS library to:

  1. Parse the .ots file into its op stream.
  2. Confirm the proof's target digest equals the event's sha256 tag (see F-C3).
  3. Validate the Merkle path to a Bitcoin block header.
  4. Check the block header (hash + height) against a Bitcoin node or API. Only then display "confirmed." Remove or relabel the byte-pattern heuristic as a non-authoritative hint.

F-C3 (Critical) — RESOLVED: The OTS proof's target digest is now bound to the event hash

Location: www/verify.html:534, www/js/pq-crypto.mjs:875.

Issue. verify.html reads the ots and sha256 tags and checks that the sha256 tag matches hashFullEvent(kind1Event) — good. But it never checks that the OTS proof's internal target digest equals that hash. isOtsConfirmed only looks for the Bitcoin magic bytes; it does not parse the proof's target. So a proof timestamping any digest would be accepted as long as it contains the magic bytes.

Impact. Even if the byte-pattern heuristic were replaced by a real Merkle-path check, a proof committing to a different digest would still be accepted unless the target digest is explicitly compared to the sha256 tag. An attacker could attach a genuine Bitcoin-anchored OTS proof for some unrelated file to a fraudulent event and have it pass.

Fix. After parsing the OTS file, extract its target digest and assert targetDigest === sha256Tag (case-insensitive hex compare) before trusting any attestation in the proof.

F-H1 (High) — No tests, no known-answer test vectors

Location: package.json:7.

Issue. package.json has "test": "echo \"Error: no test specified\" && exit 1". No tests anywhere: no BIP32 derivation known-answer tests, no PQ sign/verify round-trips, no verifyNIPQRContent accept/reject tests, no OTS prefix tests.

Impact. No regression protection; correctness rests entirely on manual review. A refactor could silently break signature verification or key derivation.

Fix. Add a test suite covering:

  • BIP32→PQ-seed derivation with a fixed mnemonic → expected seed bytes.
  • PQ keygen determinism (same seed → same pubkey).
  • PQ sign→verify round-trip for each algorithm; tampering fails.
  • verifyNIPQRContent accept (genuine) and reject (tampered content/sig/pubkey, wrong e/sha256 tags).
  • computeEventId/verifyNostrEvent against a known Nostr test vector.
  • OTS prefix construction (isDetachedOtsFile true/false).

F-H2 (High) — verifyNostrEvent does not check event.id against the computed hash

Location: www/js/pq-crypto.mjs:385.

Issue. verifyNostrEvent computes the SHA-256 of the canonical serialization and verifies the Schnorr signature against it, but never compares event.id to the computed hash. An event with a wrong/foreign id but a valid signature passes. The kind 11112 wrapper is checked only via verifyNostrEvent in verify.html:522, so a spoofed id on the wrapper is not detected.

Fix. Add:

const computedId = bytesToHex(hash);
if (event.id && event.id.toLowerCase() !== computedId) return false;

and have callers require id presence.

F-H3 (High) — XSS surface via innerHTML with relay/localStorage-derived data

Location: www/index.html:761 (setStatus), www/index.html:750 (otsNotice), www/verify.html:403 (setStatus), www/verify.html:627.

Issue. Multiple sites use element.innerHTML = \...${variable}...`with data from relays or localStorage. RelayNOTICE` messages and error strings are attacker-influenced. No CSP (F-H4) and no SRI, so injected script would execute.

Fix. Use textContent for all dynamic strings, or escape rigorously. Add a strict CSP and SRI (F-H4).

F-H4 (High) — No Content-Security-Policy and no SRI on the bundle

Location: www/index.html, www/verify.html.

Issue. Neither page emits a CSP meta tag; the bundle and /nostr-login-lite/ scripts load without integrity.

Impact. A compromised or MITM'd host can inject script that exfiltrates the seed phrase or PQ secret keys.

Fix. Serve a strict CSP; add SRI hashes; load third-party scripts only from trusted CDNs with SRI.

F-H5 (High) — isOtsConfirmed substring search can false-positive

Location: www/js/pq-crypto.mjs:875.

Issue. isOtsConfirmed scans the entire proof for the 8-byte magic. Pending attestations could contain those bytes by coincidence; a crafted proof can include them deliberately (F-C2). A proper parser would walk the OTS op stream.

Fix. Replace with a real OTS parser. (Subsumed by the F-C2 fix.)

F-M1 (Medium) — PQ signatures cover human-readable text, not a canonical binding

Location: www/js/pq-crypto.mjs:455.

Issue. The signed message is the full content string, mixing security-relevant fields (npub, block height) with display prose and hardcoded URLs. If the display text or URLs change, all existing PQ signatures would need re-issuing.

Fix. Sign a canonical structured attestation (or its hash) — e.g., JSON.stringify({v:1, npub, blockHeight, pqPubkeys}) — and include the human-readable text separately for display.

F-M2 (Medium) — block_height tag is not verified

Location: www/js/pq-crypto.mjs:465.

Issue. The block_height tag and content claim are purely informational. Nothing verifies the height was current at signing time.

Fix. Either remove the block-height security claim, or verify it against the OTS-attested block once real OTS verification is implemented.

F-M3 (Medium) — BIP32 non-hardened leaf indices used for PQ seeds

Location: www/js/pq-crypto.mjs:49.

Issue. PQ seeds at non-hardened children 1–8 under hardened parent m/44'/1237'/0'/0'. PQ seeds (child private keys) remain secret unless the parent xpub leaks. The docs now warn about this (resolved in reconciliation). No enforcement.

Fix. Document the requirement never to publish the xpub at m/44'/1237'/0'/0' (done). Consider hardened indices for PQ children if wallet-compatibility is not required.

F-M4 (Medium) — Truncation rule for concatenated BIP32 children is arbitrary

Location: www/js/pq-crypto.mjs:160.

Issue. "Take the first N bytes" is now documented (resolved in reconciliation), but a future implementer who takes the last N would break compatibility.

Fix. Document the exact rule (done). Consider HKDF-Expand on the concatenated children for a less arbitrary construction in a future version.

F-M5 (Medium) — Falcon-512 is a draft standard (FIPS 206 not finalized)

Location: www/js/pq-crypto.mjs:728.

Issue. Falcon-512 is labeled "FIPS 206 (draft)." Standardization risk; the multi-scheme design mitigates it.

Fix. Communicate Falcon's draft status in the UI. Consider deferring Falcon until FIPS 206 is finalized.

F-L1 (Low) — PQ secret keys and seed held in global scope, never zeroed

Location: www/index.html:641.

Issue. PQ secret keys and seed held in JS global scope for the session, never zeroed. A compromised page (F-H3/F-H4) can read them.

Fix. Acceptable for a demo; for production, minimize secret lifetime and clear references on sign-out.

F-L2 (Low) — 12-word default gives only ~64-bit PQ security on seed entropy

Location: www/js/pq-crypto.mjs:76, README.md (now recommends 24 words).

Issue. generateSeedPhrase defaults to 128 bits (12 words). Docs now recommend 24 words; app still defaults to 12.

Fix. Default to 24 words, or warn that 12-word gives only ~64-bit PQ security on the seed entropy.

F-L3 (Low) — hexToBytes/base64ToBytes do not validate input

Location: www/js/pq-crypto.mjs:362, www/js/pq-crypto.mjs:341.

Issue. No input validation; malformed input produces NaN bytes or throws late. The verify page accepts pasted event JSON from users.

Fix. Validate input format and length; throw a clear error on malformed input.

F-L4 (Low) — e tag validation does not check kind1Event.id equals computed id

Location: www/js/pq-crypto.mjs:624.

Issue. The e tag validation checks against the computed kind 1 id and kind1Event.id if present — but does not check that kind1Event.id itself equals the computed id.

Fix. Assert kind1Event.id (if present) equals the computed id, or ignore kind1Event.id and rely on the computed id.

F-L5 (Low) — timestampEvent sends binary body with wrong Content-Type

Location: www/js/pq-crypto.mjs:798.

Issue. Content-Type: application/x-www-form-urlencoded with a raw binary body. Should be application/octet-stream. Works in practice.

Fix. Use Content-Type: application/octet-stream (or omit the header).

F-L6 (Low) — Relay publishing treats ws.onclose as success

Location: www/index.html:1139.

Issue. publishToRelays treats ws.onclose as success. A relay that closes without sending OK is counted as successful.

Fix. Only count as success on an explicit OK message.

F-L7 (Low) — App depends on server-provided /nostr-login-lite/ scripts not in the repo

Location: www/index.html:606, www/verify.html:346.

Issue. The pages load /nostr-login-lite/nostr.bundle.js and nostr-lite.js from absolute paths. These files are not in the repo.

Fix. Vendor the scripts into the repo (with SRI) or document the deployment dependency.

F-L8 (Low, NEW) — why_what_how.md Component 4 summary still uses successor-model language

Location: why_what_how.md:64.

Issue. The six-components summary for Component 4 says: "The old identity signs to authorize the migration; the new identity signs to endorse the PQ keys." This is successor-model language describing the unimplemented two-account flow. Component 4 is marked "Design only" in the README, but why_what_how.md does not carry that marker, so a reader could mistake this for an implemented feature.

Fix. Either add a "design only" note to why_what_how.md Component 4, or reword to avoid implying the successor signature is implemented.

Reference in New Issue View Git Blame Copy Permalink