docs: Record security audit verdict

docs/audits/security-2026-09-22.md

@@ -0,0 +1,213 @@
+# Security audit verdict — 2026-09-22
+
+Scope: python-codex32 at
+[`c118a834fc9cce95ead3c05b3a2532e1320980c5`](https://github.com/BenWestgate/python-codex32/commit/c118a834fc9cce95ead3c05b3a2532e1320980c5),
+the graphical wallet flow at
+[`78f4a3b4e99b02bf012ff179aaa7f3f8a96683f2`](https://github.com/BenWestgate/python-codex32/commit/78f4a3b4e99b02bf012ff179aaa7f3f8a96683f2)
+and its current descendant
+[`8f1001f61983cf60023120af4b01bbc99d6f3a84`](https://github.com/BenWestgate/python-codex32/commit/8f1001f61983cf60023120af4b01bbc99d6f3a84),
+and the BIP93/Codex32 assumptions those paths rely on.
+
+This document records the security-review result. It is not a replacement for
+the normative [security model](../security/model.md) or
+[security invariants](../security/invariants.md).
+
+## Verdict
+
+The core implementation is careful about parsing, checksum validation,
+reconstruction, Bitcoin Core process boundaries, and secret transfer. No break
+of the Codex32 checksum, finite-field reconstruction, OS-CSPRNG use, or BIP32
+root validation was found.
+
+The audit did find two medium library implementation defects, one low
+interoperability/specification-divergence defect, and a wallet recovery ordering
+defect. It also confirmed two properties of BIP93 that applications must not
+mistake for cryptographic authentication: the 20-bit set identifier does not
+prevent cross-set mixing, and checksum-valid shares do not authenticate the
+intended reconstructed secret against active substitution.
+
+The release-gate rule for callers is therefore stricter than “valid Codex32
+shares reconstruct a valid seed”: before recovered key material mutates a live
+wallet, the operator must establish that the recovered seed is the wallet they
+intend using the strongest independent evidence they actually have. For the
+current single-sig restore flow this is an accident-safety check: normally a
+typed wallet-record fingerprint; when no independent record exists, the product
+instead makes that absence explicit and requires a last-resort visual
+confirmation before the operator authorizes the restore. That no-record path is
+not independent authentication of the recovered seed. Stronger resistance to
+malicious share/seed replacement is a separate problem, tracked as an
+authenticated encrypted descriptor backup in issue #55.
+
+## Confirmed implementation findings
+
+### Medium — unbounded correction alignment cache
+
+Tracking: [#21](https://github.com/BenWestgate/python-codex32/issues/21).
+
+- Affected component: `src/codex32/correction.py`, `_syndrome_alignment`.
+- Failure scenario: caller-controlled HRP/length combinations create distinct
+  entries in a process-global unbounded cache. A long-lived GUI or service can
+  retain alignment tables monotonically.
+- Property violated: bounded resource use for untrusted correction input.
+- Exploitability: practical for long-lived processes. A local reproduction with
+  50 attacker-distinct HRP keys retained about 5.66 MiB, approximately 113 KiB
+  per key in that run.
+- Remediation: remove attacker-specific global caching or use a small bounded
+  LRU with lifecycle clearing; add churn tests that assert a hard bound.
+- Confidence: high.
+
+### Medium — secret-bearing default string representations
+
+Tracking: [#22](https://github.com/BenWestgate/python-codex32/issues/22).
+
+- Affected component: artifact string/repr paths and nested correction
+  candidates.
+- Failure scenario: `_Artifact.__str__` returns the complete Codex32
+  secret/share text, while default object representations can retain or expose
+  secret-bearing fields. Generic logging, notebooks, REPLs, assertion output,
+  or exception context can persist recovery material unexpectedly.
+- Property violated: secrets stay out of ordinary output and logs unless an
+  explicit reveal/export operation is requested.
+- Exploitability: practical accidental disclosure; no attacker is required once
+  a secret-bearing object reaches generic diagnostics.
+- Remediation: make normal `str`/`repr` redacted and require an explicit
+  reveal/export operation for the full Codex32 string; test nested containers
+  and correction candidates.
+- Confidence: high.
+
+### Low — overlong BIP173 human-readable parts are accepted
+
+Tracking: [#32](https://github.com/BenWestgate/python-codex32/issues/32).
+
+- Affected component: `src/codex32/bip93.py` decoder and public correction
+  contexts.
+- Failure scenario: an otherwise checksum-valid Codex32 string with an HRP of
+  84 or more characters is accepted even though BIP173 limits the HRP to 83
+  characters. The same invalid namespace can be admitted to correction setup.
+- Property violated: accepted encodings must remain inside the normative
+  BIP173/BIP93 grammar so independent implementations agree on validity.
+- Exploitability: practical interoperability failure, not a secret compromise.
+  A concrete 84-character-HRP string was accepted during the audit.
+- Remediation: reject HRPs longer than 83 characters in decoding and correction
+  context validation; cover the 83/84-character boundary in tests.
+- Confidence: high.
+
+### Medium — wallet restore verifies recovered identity after mutation
+
+Tracking: GUI issue #26; CLI issue #30.
+
+- Affected component: GUI recovery in `src/codex32_gui/pages.py`, CLI recovery
+  in `src/codex32/cli.py`, and the shared mutating wallet initialization path.
+- Failure scenario: both supported restore front ends mutate Core before the
+  recovered wallet identity is checked. The GUI calls `wallet_setup.fill(...)`
+  before it computes and displays the identity. `ms32 wallet` calls
+  `core.initialize(...)` before displaying the recovered fingerprint. A wrong
+  but checksum-valid recovered seed can therefore import private descriptors
+  into the selected empty Bitcoin Core wallet before the operator can detect
+  the mismatch.
+- Property violated: the operator must identify the intended wallet before
+  recovered key material changes durable wallet state.
+- Exploitability: practical for accidental wrong/mixed cards, miscorrection, or
+  another valid-but-unintended recovery. Deliberate threshold-share replacement
+  is a separate threat because an attacker who can replace a threshold can
+  already learn the seed; issue #55 tracks authenticated descriptor evidence for
+  that stronger threat model.
+- Remediation: on every restore path, derive wallet identity without importing
+  descriptors. Prefer a verified encrypted descriptor backup when available;
+  otherwise require a matching wallet-record fingerprint (and later optional
+  single-sig descriptor checksum), or make the no-record visual confirmation an
+  explicit operator choice. Only then call the mutating wallet initialization
+  path.
+- Confidence: high. Reconfirmed against the Bails-pinned GUI commit, the current
+  GUI descendant, and the supported `ms32 wallet` CLI path.
+
+### Threat-model disposition — 32-bit fingerprint is not a malicious-tampering defense
+
+Tracking: closed issue #27; stronger descriptor authentication is issue #55.
+
+- Affected component: wallet record/recovery identity design across restore
+  front ends.
+- Observation: the four-byte BIP32 master fingerprint is intentionally a
+  human-scale identifier. A party choosing substitute seeds can grind a matching
+  value; the 20-bit backup identifier is weaker still.
+- Release-gate disposition: this does not invalidate the fingerprint as an
+  accident-safety gate. Its job in #28/#57 is to catch wrong/mixed cards,
+  miscorrection and transcription mistakes before Core is changed. It is not
+  represented as authorization against a malicious party able to replace a
+  threshold of shares.
+- Stronger design: issue #55 authenticates the full single-sig descriptor backup
+  with material derived from the recovered seed, verifies that the recovered
+  seed reproduces those descriptors before wallet mutation, and relies on
+  independently stored copies of that backup for tamper resistance.
+- Human record follow-up: #43 adds checksummed/type-back record fields and may
+  accept a canonical single-sig descriptor checksum as another second-class
+  accident check.
+- Confidence: high on the distinction between the two threat models; the exact
+  encrypted-backup format remains design work in #55.
+
+## Confirmed BIP93/Codex32 application hazards
+
+These are protocol properties, not evidence that python-codex32 implemented the
+specified arithmetic incorrectly.
+
+The current BIP93 text already states that identifiers are for disambiguation
+and that the checksum does not protect against maliciously constructed errors.
+These findings therefore require application-side enforcement and documentation,
+not an upstream BIP93 vulnerability report.
+
+### Medium — 20-bit set identifier does not prevent cross-set mixing
+
+- Affected specification: BIP93 set identifier and threshold recovery model.
+- Failure scenario: shares from two valid sets with the same compatible header
+  can be combined into the required threshold and interpolate a third seed that
+  is checksum-valid. The audit reproduced this behavior.
+- Property violated: if an application assumes the identifier authenticates
+  set membership, that assumption fails.
+- Exploitability: practical once shares from compatible colliding/matched sets
+  are available. The 20-bit identifier is a typo/mix-up aid, not a
+  cryptographic set commitment.
+- Remediation: for accidental mix-ups, require the operator to identify the
+  intended wallet before import using the strongest evidence they actually
+  possess. Do not describe the identifier itself as authentication.
+- Confidence: high.
+
+### Medium — valid-share substitution can force an attacker-chosen reconstruction
+
+- Affected specification: unauthenticated threshold-share model.
+- Failure scenario: an attacker who can replace enough shares can construct
+  checksum-valid replacement shares that make the victim reconstruct a chosen
+  target secret. The audit reproduced this for a 2-of-2 set.
+- Property violated: share integrity/provenance and intended-secret
+  authentication when active substitution is in scope.
+- Exploitability: practical when the attacker can replace the required share
+  material. This is not a secrecy break; such an attacker can already recover
+  the original seed from the threshold material they control.
+- Remediation: keep the release-gate fingerprint/no-record UX scoped to accident
+  safety. Where malicious substitution is in scope, authenticate the complete
+  descriptor backup independently as designed in #55 and keep copies apart from
+  the recovery cards.
+- Confidence: high.
+
+## Falsified candidate
+
+The earlier “opaque-HRP deadline bypass” candidate was falsified during review
+and is not a security finding. It must not be cited or tracked as one.
+
+## Required recovery invariant
+
+For every wallet restore path built on python-codex32:
+
+> Before any recovered key material mutates a wallet, the operator must establish
+> the intended wallet using the strongest independent evidence they actually
+> have: a verified seed-keyed descriptor backup when available; otherwise a
+> matching wallet-record fingerprint and/or canonical single-sig descriptor
+> checksum. If no independent wallet record exists, the restore must state that
+> identity cannot be authenticated and require an explicit last-resort visual
+> confirmation of the recovered fingerprint and codex32/Bails identifier result
+> before the operator authorizes mutation.
+
+The fingerprint and descriptor-checksum paths are human-scale accident-safety
+checks. The no-record path is an explicit accident-safety waiver/authorization,
+not independent authentication evidence. Malicious share/seed replacement is a
+separate threat model and uses the authenticated encrypted descriptor backup
+tracked in #55.