mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-10-27 16:02:05 +00:00
thePR0M3TH3ANandClaude Fable 5
ee330c52f7
Four independent reviewers went over the branch. The worst findings were in
code I had just written and reviewed myself, which is the argument for
having done this.
CRITICAL
- A mnemonic was validated in one form and derived from in another. @scure's
normalize only counts words, so a phrase with a trailing newline passed
validation and then derived a completely different seed, while Python
canonicalizes and derives the correct one. The result was a vault encrypted
under a key that neither implementation -- nor the user's written-down
phrase -- could ever reproduce. Mnemonics are now canonicalized (NFKD,
lowercase, single-space) once, and every derivation site uses that form:
Bip85.fromMnemonic, deriveIndexKeyBytes, deriveTotpSecret, deriveKeyIndex
and generateFingerprint.
- atomicWrite ignored short writes. `handle.write()` may write fewer bytes
than asked (ENOSPC, RLIMIT_FSIZE, EINTR); the truncated temp file was then
fsynced and renamed over the only copy of the vault, with exit code 0 and
no stderr -- inside the function whose comment promised crash safety. It
now loops until every byte lands, fails loudly otherwise, and removes the
temp file on any failure.
- `fingerprint add` overwrote an existing profile. It consulted only
fingerprints.json, which readFingerprints returns empty for whenever the
file is corrupt or unreadable, then rewrote parent_seed.enc and the index.
Restoring a profile directory from backup and re-registering it -- the
canonical recovery gesture -- destroyed the vault. It now refuses if the
seed or index file already exists.
- Entry-id allocation could collide. A non-numeric or above-2^53 key (which
can arrive through import or a merge) made nextIndex return NaN or stop
incrementing, and insert overwrote blindly, so each new entry destroyed the
previous one. Both are now rejected, and insert refuses to overwrite.
HIGH
- RelayPool accepted any event a relay chose to send: verifyEvent only proves
an event is internally consistent, never that it came from the pubkey,
kind, or id we asked for. Events are now structurally validated (including
created_at being a finite safe integer -- "1e999" parsed to Infinity and
sorted ahead of everything forever) and matched against the filter.
- Snapshot handling had no resource budget: a decompression bomb turned 20 KB
of relay data into 20 MB, unbounded. Chunk count, chunk size, total
compressed and total decompressed bytes are now capped, the gunzip is
aborted mid-stream when it exceeds the budget, and each chunk must match
the size its signed manifest declares.
- Manifest tie-breaking used relay arrival order, letting a relay pick which
of two same-second snapshots won; it now breaks ties on event id, which no
party can steer. Falling back to an older manifest is recorded so callers
can surface a downgrade.
- Argon2/PBKDF2 parameters were read unbounded from the kdf block, which sits
outside the AEAD and is writable by anyone with directory access: 1e9 time
cost hung indefinitely, and this core targets browsers too. Now clamped.
- Config and fingerprint-registry writes held no lock at all, losing 4 of 6
concurrent updates in testing. Both now use a read-modify-write lock, and
the vault lock itself only breaks a lock whose owner pid is gone, steals it
atomically by rename, and releases only if it still owns it.
- Plaintext egress (document export, vault export) used writeFile's mode,
which applies only on creation, and followed symlinks -- an existing
world-readable file or a planted symlink received the secret. Both now go
through atomicWrite, which creates a fresh 0600 inode.
MEDIUM
- canonicalJson did not match Python. Exponent padding (1e-7 vs 1e-07), the
1e16 threshold, -0, and key ordering (UTF-16 code units vs code points) all
differed, and the code comment claiming they agreed was false. Since entry
hashes decide merge winners, two clients could keep different versions of
an entry forever. Now matched, with 24 differential fixtures generated from
CPython, and values above 2^53 refused rather than guessed at.
- "__proto__" as an entry or tombstone key silently vanished and polluted the
prototype; containers are now null-prototype. Python truthiness differs
from JS on empty containers, so `"_deleted": []` tombstoned an entry in JS
and kept it in Python; now matched.
- Policy blocks with JSON null produced the literal string "null" as the
special-character alphabet. TOTP secrets that pyotp accepts (lowercase,
unpadded) were rejected. Both fixed.
- hexToBytes accepted malformed input ("1z" -> 0x01, "-1" -> 0xff) where
Python raises; relay-supplied ids and signatures reach it.
- Password/fingerprint normalization used JS trim(), which strips a different
set than Python's strip(), changing derived keys; now uses Python's set.
Also removes a scratch file a reviewer's session left committed.
220 core + 71 CLI tests green; 36/36 cross-implementation checks green.
Co-Authored-By: Claude Fable 5 <[email protected]>