Writer Security Audit
Scope: packages/chain/src/.sol, packages/web/src/utils/{utils,signer,keyCache,entryCache}.ts,
packages/server/src/{server,helpers,middleware,privy,relay,routes/}.ts. Indexer and DB layer not audited in depth.
Summary: Two critical issues to fix before launch (encryption key derivation is phishable; signature replay protection can be bypassed
via ECDSA malleability). Several high-severity issues around an unauthenticated relay that can be drained by an attacker. Detailed
findings below, ordered by severity.
CRITICAL
✅ C-1. Encryption key derivation is not domain-bound (phishable)
ℹ️: Fixed via Option B — added getDerivedSigningKeyV4 in signer.ts that uses
EIP-712 typed data with the writer's frozen storage_id in the message
body, then HKDF-SHA256 → 32-byte AES-256-GCM. The domain intentionally
omits both chainId and verifyingContract so the derivation is
chain-portable AND survives chain migrations / contract redeploys (the
storage_id is what matters, not the live contract address). Schema +
migrations 0006/0007 add storage_id to writer + entry tables, frozen at
insert. Read paths (hooks.ts, Entry.tsx, SavedView, page.tsx) detect the
enc:v4:br: prefix and fetch the per-writer v4 key. Write paths
(EntryListWithCreateInput, Entry.tsx handleSave) always emit v4. The
MigrateModal upgrades v1/v2/v3 → v4 on user opt-in. v1/v2/v3 read paths
remain intact for backward compat. Also pulls in M-1 (now AES-256, not
AES-128). Does NOT yet address H-1 (no AAD binding to entry id) — that
remains a separate follow-up.
Files: packages/web/src/utils/signer.ts:209-279
const message = "Writer: write (privately) today, forever.\n\nNOTE: Only sign this message on https://writer.place.";
const signature = await provider.request({ method: "personal_sign", params: [encodedMessage, wallet.address] });
const hash = keccak256(signature);
return key.slice(0, 16);
The encryption key is derived from personal_sign(fixedString). EIP-191 personal_sign has no domain binding — no chain id, no contract, no
origin. Any malicious dApp/wallet popup that asks the user to personal_sign the exact same string will produce the same signature, and
therefore the same AES key. The attacker can then decrypt every "private" entry the user has ever written, retroactively, by reading the
ciphertext from the public chain.
The v3 mitigation ("only sign on https://writer.place") is a textual warning inside the message body. It is not a security boundary:
- Most users do not read sign-message popups carefully
- Wallets render the warning differently or wrap text
- A malicious site can replicate the warning in any way it wants — there is no programmatic check
- The same risk exists for other dApps prompting personal_sign with copied messages
This breaks the central privacy promise of Writer. Encrypting in the browser is meaningless if anyone who tricks the user into one
signature can decrypt the entire archive.
Fix: Derive the encryption key via EIP-712 typed data so it is bound to chain id + a verifying contract (or at least to an EIP-712 domain
that other sites cannot reuse). Even better: bind to the specific writer contract address so each "place" has a distinct key, limiting
blast radius. Example shape:
const sig = await provider.request({
method: "eth_signTypedData_v4",
params: [address, JSON.stringify({
domain: { name: "Writer Encryption", version: "1", chainId, verifyingContract: writerAddress },
primaryType: "DeriveKey",
types: { EIP712Domain: [...], DeriveKey: [{ name: "purpose", type: "string" }] },
message: { purpose: "private-entry-encryption" },
})],
});
Then HKDF-Expand the signature into the AES key. EIP-712 domain separation makes signatures not reusable on other sites/contracts.
✅ C-2. Signature replay protection can be bypassed via ECDSA malleability
Files: packages/chain/src/Writer.sol:50-54, packages/chain/src/VerifyTypedData.sol:32-52, packages/chain/src/ColorRegistry.sol:23-32
function _validateAndMarkSignature(bytes memory signature) internal {
bytes32 signatureHash = keccak256(signature);
require(!signatureWasExecuted[signatureHash], "Writer: Signature has already been executed");
signatureWasExecuted[signatureHash] = true;
}
Replay protection is keyed off keccak256(signature_bytes). ECDSA signatures are malleable: for any valid (r, s, v) signing a digest, (r, n - s, v ^ 1) is also a valid signature for the same digest and recovers the same signer. The contract:
- Does not enforce low-S in _getSigner (VerifyTypedData.sol:32) — it only normalizes v < 27.
- Uses ecrecover directly (which accepts both S forms).
- Hashes the raw signature bytes for the replay set, so the malleated signature has a different signatureHash and passes the
!signatureWasExecuted check.
Impact: Any third party who observes a signed payload (in the mempool, in your relay logs, in the DB, in network traffic) can submit a
malleated copy exactly once and re-execute the operation:
- createWithChunkWithSig → duplicate entry created with the same content under the original author (most concerning; pollutes a user's
timeline with content they didn't intend to write twice). - removeWithSig, updateWithSig → idempotent in practice but still wasteful.
- setHexWithSig → idempotent.
- addChunkWithSig → can attempt re-add but the chunk-already-filled check (WriterStorage.sol:110) blocks it.
This is exploitable by anyone — including your own relay operator — without the user's consent.
Fix: Two complementary changes:
- Enforce low-S in _getSigner:
if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) return address(0); - or use OpenZeppelin's ECDSA.recover (you already import OZ).
- Index the replay map by the digest (keccak256("\x19\x01" || domainSeparator || structHash)) or by (signer, nonce), not by the
signature bytes. The digest is unique regardless of malleation. This is the canonical pattern used by EIP-2612, OZ's Nonces, etc.
HIGH
H-1 has been reclassified to MEDIUM and moved below — see entry after M-1. The cross-writer half of this finding is already mitigated by the C-1 fix (per-storage_id keys), which downgraded the practical severity.
✅ H-2. /factory/create is fully unauthenticated — attacker can drain the relay wallet
ℹ️: Fixed by adding requireWalletAuth middleware to /factory/create. The
route handler also asserts admin === c.var.walletAddress, so the
authenticated user can only create writers with themselves as admin
(closes the confused-deputy attack). Per-user rate limiting is still a
follow-up but the open-relay-drain hole is closed. The endpoint was
also removed from the public /docs page and DOCS.md so external
callers no longer find it documented.
Files: packages/server/src/routes/writer.ts:169-217, packages/server/src/relay.ts
.post("/factory/create", factoryCreateJsonValidator, async (c) => {
...
const { wallet, nonce: relayNonce } = await relay.sendTransaction({...});
Anyone with a curl can hit this endpoint and the server will pay gas to deploy a Writer + WriterStorage pair through your relay. Each
call costs hundreds of thousands of gas and is permanent. There is no Privy auth, no rate limit, no captcha, nothing. An attacker can
drain your relay wallet balance in a few minutes and pollute your DB with thousands of garbage writers.
Fix: Require Privy auth on this endpoint (the user creating the writer presumably has a Privy session). At minimum apply per-IP and
per-Privy-user rate limiting. Consider requiring the user to pay gas themselves for /factory/create, since this is the highest-cost
operation.
✅ H-3. All sig-relayed write endpoints are unauthenticated — relay funds can be ground down
ℹ️: Fixed by adding requireWalletAuth middleware to /color-registry/set,
/writer/:address/entry/createWithChunk, /writer/:address/entry/:id/update,
and /writer/:address/entry/:id/delete. Each route handler asserts
getAddress(recoveredSigner) === c.var.walletAddress after recovering
the EIP-712 signer — so even if an attacker captures someone else's
signature, they can't authenticate as that wallet to submit it. The
four frontend write API helpers (setColor, createWithChunk, editEntry,
deleteEntry) and the factoryCreate helper now all forward a Privy
bearer token; all 8 call sites updated. The endpoints were also stripped
from /docs and DOCS.md. Per-IP rate limiting is still a follow-up but
the captured-signature replay + anonymous relay-drain attack class is
closed.
Files: packages/server/src/routes/writer.ts:241,348,433 (createWithChunk, update, delete), :127 (color-registry/set)
These endpoints accept a signature, recover the signer, and submit to the chain via the relay. There is no Privy auth and no rate
limiting. Several attack patterns:
- Captured-signature replay against the server. If an attacker observes any valid signature (e.g., it's stored in syndicate_tx/relay
logs that are queryable, or visible in mempool, or you have a security bug elsewhere), they can resubmit the same payload to your
endpoint over and over. The first onchain attempt will succeed; every subsequent attempt will revert at signatureWasExecuted — but the
relay still pays gas for the reverted transaction. Combined with C-2, the attacker even gets one extra free replay. - Forged-but-recoverable signatures. Anyone can compute valid signatures from their own wallet that recover to themselves but lack
WRITER_ROLE on a target Writer contract. The contract reverts, but again the relay pays gas. The attacker can do this against any known
writer address on a loop. - No request shape limits beyond the single-field max. The default Hono body limit + zod cap is 500k chars per content field. An
attacker can send 500KB payloads at high QPS.
Fix:
- Add requireWalletAuth (Privy) middleware to every write endpoint AND require walletAddress === recoveredSigner. This eliminates the
open relay completely. - Add per-IP and per-wallet rate limits.
- For createWithChunk/update/delete: also verify the signer is in the writer's managers list in the DB before relaying, so a
forged-from-attacker-wallet signature is rejected before it ever reaches the chain.
❌ H-4. /manager/:address and /relay/wallets leak info without auth
ℹ️: Intentional :), all data is visible onchain & our encryption mechanism protects against from actors reading private content
Files: packages/server/src/routes/writer.ts:39-47, packages/server/src/routes/relay.ts:6-17
- GET /manager/:address returns the full list of writers managed by any address along with all entries. Trivially enumerable and reveals
which wallet authored which "place" — including entries flagged private (the ciphertext is exposed, so it's still subject to
brute-forcing if any of the encryption mitigations fail). - GET /relay/wallets exposes your relay wallet addresses + balances. This makes it cheap to monitor when your relay funds are running low (and to time abuse with H-2/H-3).
Fix: Gate these endpoints behind the same auth used by /saved/:userAddress/.... Don't return entries from /manager/:address to
non-owners. Move /relay/wallets behind assertAdminKey.
MEDIUM
✅ M-1. AES-128 instead of AES-256 ℹ️: Fixed as part of C-1. v4 derivation uses HKDF-SHA256 to derive a 32-byte key, which Web Crypto's AES-GCM importKey treats as AES-256-GCM. v1/v2/v3 still use AES-128 (16-byte truncated keccak) but those are read-only legacy paths.
Files: packages/web/src/utils/signer.ts:230,255,275
return key.slice(0, 16); // Use the first 16 bytes for a 128-bit key
The signature is hashed to 32 bytes, then truncated to 16. AES-128-GCM is still safe today, but you're paying the cost of a 256-bit
derivation and throwing half of it away. Switch to a 32-byte key (AES-256-GCM). While you're touching this, run the signature through
HKDF rather than a single keccak truncation, so you can derive multiple subkeys (e.g., per-writer) without re-prompting the user.
❌ H-1 → reclassified to M. Encryption has no AAD — server can swap ciphertexts between entries ℹ️: Won't fix at launch. Originally rated HIGH; downgraded to MEDIUM after the C-1 fix because the cross-writer half of this attack is now structurally impossible.
What C-1 fixed: each Writer has its own per-storage_id v4 key. A
ciphertext encrypted under writer A's key cannot be decrypted under
writer B's key — AES-GCM throws a tag mismatch. So a server cannot
move encrypted entries BETWEEN writers. The cross-place leak scenario
described in the original audit is closed.
What remains: same-writer ciphertext substitution. A compromised
server could swap entry 5 and entry 6 within the same writer (both
encrypted with the same per-writer key). The user's client would
decrypt both successfully and display each piece of content under
the wrong entry id. The attack requires write access to the API/DB,
which is the same trust boundary as any hosted web app.
Why we accept this:
- Threat model: requires a compromised or malicious *server*, not
an external attacker. That's the same trust boundary as every
non-zero-trust hosted application.
- Architecture: the API is a caching layer over chain events. The
chain is the source of truth. Anyone who doesn't trust the API
can run their own Ponder indexer against the factory address or
read entries directly from chain via eth_call. The escape hatch
exists by design.
- Project promise: "write today, forever" is anchored on the chain,
not on trusting the server. The encryption claim is "encrypted
at rest with a key the host can't read" — that claim remains
true. We are not claiming "and the host can't even rearrange
the encrypted blobs."
- Public message board entries are stored as plaintext, so this
finding does not apply to the launch public writer at all.
When to revisit:
- If we ever offer shared / multi-author writers where authors don't
trust each other but trust the host
- If we ever onboard contributors with prod DB access
- If we change the encryption claim to a stronger one (full E2EE)
- If we add public-facing trust statements about content immutability
beyond what the chain enforces
Possible future fix paths (none planned):
- Bind AAD to the user's signature bytes (Option 2 from the design
discussion); requires the frontend to fetch / verify the signature
out-of-band, not just trust the API.
- Have the frontend verify entry content against chain on first
load.
- Build a chain-only frontend variant for high-trust users.
Files: packages/web/src/utils/utils.ts:191-245
encrypt/decrypt use AES-GCM with a random 12-byte IV but no additional authenticated data (AAD). Because the same wallet uses one key across all of its private entries (note: per-writer post C-1, so the cross-writer attack is closed), the server (or anyone with write access to the DB / on-chain entries) can swap ciphertext between entries WITHIN the same writer, and the client will happily decrypt them, attribute the decrypted text to the wrong entry, and display it as authentic.
For a journaling product, this lets a malicious server show the user "yesterday's entry" when they ask for today's within the same writer.
Fix (deferred): Bind AAD to the entry's identity at encrypt time:
const aad = new TextEncoder().encode(writer:${storageAddress.toLowerCase()};entryId:${onChainId});
crypto.subtle.encrypt({ name: "AES-GCM", iv, additionalData: aad }, cryptoKey, message);
For new entries (where onChainId isn't yet known) bind to a content-derived id or wait until creation, then re-encrypt; alternatively
bind to (storageAddress, nonce) from the signed payload. Note: requires another version bump (enc:v5:) and a migration story.
❌ M-2. Compression-then-encryption (length side channel) ℹ️: Won't fix at launch. The CRIME-class attack this finding is concerned with requires attacker-injected plaintext to be mixed into the victim's encryption operation. Writer's current model precludes that: - Each entry is encrypted by a single author. The author writes 100% of the bytes that get compressed + encrypted. No third-party content is folded into a victim's blob. - Public message board entries are stored as plaintext, so this finding does not apply to them at all. - There is no oracle to query repeatedly — each entry is a one-shot encryption.
Residual leak (acknowledged, accepted): unpadded ciphertext means
anyone reading the chain can infer the approximate length of each
entry. This is metadata leakage in the same category as on-chain
timestamps and tx patterns — already public, not something the
encryption layer is trying to hide. Padding to fixed bucket sizes
would add gas cost on every entry, which is at odds with the
permanence-first design.
REVISIT if you ever add ANY of the following features, because they
change the threat model from "no attacker injection possible" to
"attacker can probe via injected content":
- Inline comments under entries (where the comment is appended to
the parent entry's content and re-encrypted as one blob)
- Shared writers where multiple authors contribute to a single
encrypted entry
- Quote-replies where one entry's plaintext is mixed into another's
at encrypt time
- Encrypted titles / tags / mentions where another user can
influence the input
- Drafts that auto-merge attacker-controlled metadata into the
user's plaintext
If you build any of these, the right fix is NOT padding. It's:
encrypt each contributor's content as a separate AES-GCM blob with
its own key derivation, so there's no shared compression context
between authors. The mitigation is structural — design the
multi-author feature so each author's plaintext is encrypted in
isolation.
Files: packages/web/src/utils/utils.ts:170-189 and where it's used
Brotli compression happens before AES-GCM. An observer who can submit content adjacent to a victim's content (not really possible in a
single-user journaling product, but possible if you ever add comments/replies) could mount a CRIME-style attack by watching ciphertext
length. For private journaling this is low risk in practice, but worth noting if the threat model evolves. If you keep
compression-before-encryption, document the assumption that no attacker-controlled data is ever mixed into a victim's plaintext.
❌ M-3. Writer.setStorage() is admin-mutable — undisclosed centralization risk
ℹ️: admin = WRITER_ROLE generally for all
File: packages/chain/src/Writer.sol:79-82
function setStorage(address storageAddress) external onlyRole(DEFAULT_ADMIN_ROLE) {
store = WriterStorage(storageAddress);
The admin of a Writer can swap the underlying storage to a contract they control — replacing the visible history with arbitrary data.
This is a centralization risk that should be disclosed in user-facing docs ("the admin of a place controls history"). For self-owned
places this is fine; for shared places it is meaningful trust.
Mitigations: remove setStorage entirely (storage was bound at construction; pair is immutable) or make it a 2-step admin handoff so users can detect malicious swaps via events.
❌ M-4. replaceAdmin has no two-step handoff
ℹ️: no
Files: packages/chain/src/Writer.sol:74-77, packages/chain/src/WriterStorage.sol:53-56
A typo in replaceAdmin(0xWRONG) immediately and irrecoverably hands admin to the wrong address. Use OpenZeppelin's Ownable2Step-style
accept pattern (grant, then require the new admin to call acceptAdmin).
✅ M-5. Random 32-bit nonce + birthday bound ℹ️: Fixed by upgrading getRandomNonce() to 53 bits of entropy — the maximum that fits in a JS Number safely (Number.MAX_SAFE_INTEGER === 2^53 - 1). Birthday collision threshold goes from ~2^16 (~65k entries per user) to ~2^26.5 (~95M entries per user), well beyond any realistic write volume.
Stayed inside Number rather than upgrading to bigint because the
nonce flows through the JSON wire format (frontend → server → relay
→ calldata) and is currently typed as `number` throughout. 53 bits
is plenty given the actual threat model and avoids cross-layer
type-plumbing churn. If a stronger nonce ever becomes necessary, the
follow-up is to migrate the entire pipeline to bigint.
File: packages/web/src/utils/signer.ts:204-206
function getRandomNonce() {
return Number(crypto.getRandomValues(new Uint32Array(1))[0]);
}
Only 32 bits of entropy. Collisions become probable (~1%) after ~9k entries from the same user. Collision impact is limited because the
content of each signature differs and signatureWasExecuted is keyed off the signature bytes (or, after the C-2 fix, off the digest
including content). Still, prefer crypto.getRandomValues(new BigUint64Array(2)) for 128 bits, or use a strict per-user counter.
❌ M-6. WriterStorage.update allows incomplete entries by design
ℹ️: it's on the caller to write chunks correctly
File: packages/chain/src/WriterStorage.sol:115-136
update(id, totalChunks, content) clears chunks, sets totalChunks = N, but only writes chunk 0. If totalChunks > 1 the entry is left in an
"incomplete" state forever (no follow-up addChunk is required by the contract). The frontend currently always passes totalChunks = 1, so
this is dormant — but the contract is misleading and a future caller could create a permanently broken entry. Either remove the
totalChunks parameter from update or require totalChunks == 1 (and revert otherwise).
❌ M-7. WriterStorage.addChunk allows out-of-order writes / partial entries
ℹ️: writing out of order should be supported, we can't guarantee which will hit the chain first
File: packages/chain/src/WriterStorage.sol:105-113
addChunk only checks the requested index is empty, not that earlier chunks have been filled. A writer can publish chunk[5] without ever
publishing chunks[0..4], leaving the entry permanently incomplete. The completion event EntryCompleted only fires when receivedChunks ==
totalChunks, so consumers checking that flag are safe — but getEntryContent will silently concatenate empty strings with " " separators,
returning misleading data.
❌ M-8. getEntryContent joins chunks with a literal space
ℹ️: simple helper that generally is not used
File: packages/chain/src/WriterStorage.sol:154-167
Concatenation uses " " between chunks. Words at chunk boundaries get a stray space inserted, and trailing/leading whitespace is
destroyed. Lossy transformation of user content. If you keep multi-chunk support, join with the empty string and have the writer split
content carefully — or remove this helper and let the frontend reassemble (the frontend already does this).
✅ M-9. _validateAndMarkSignature ordering inside modifiers ℹ️: Fixed during the L-1/L-2/M-9 cleanup. Replaced the modifier-based pattern (signedByWithRole / signedByAuthorWithRole) with two internal helper functions (_verifyAndMark / _verifyAndMarkAuthor) that return the recovered signer. All 6 WithSig functions now compute the structHash exactly once, recover the signer exactly once, and pass it directly into store. — no more double recovery, no more modifier-vs- body drift surface. Gas savings ~13k per *WithSig call (verified via forge test gas snapshots).
File: packages/chain/src/Writer.sol:29-42
In signedByAuthorWithRole, the role check (hasRole(role, signer)) happens before _validateAndMarkSignature. If the role check reverts,
the nonce is not consumed — fine. But also note: getSigner is called both in the modifier and again in the function body of every
*WithSig function. That is wasted gas and an opportunity for inconsistency if the two call sites ever drift. Compute once, pass into the
body via a local variable, or change the modifier to write the recovered signer into a transient slot.
LOW / INFORMATIONAL
✅ L-1. DOMAIN_NAME/DOMAIN_VERSION are mutable storage variables
ℹ️: Fixed during the L-1/L-2/M-9 cleanup. DOMAIN_NAME, DOMAIN_VERSION,
and WRITER_ROLE are now string public constant / bytes32 public constant in both Writer.sol and ColorRegistry.sol. The fragile
"child state vars must initialize before the parent constructor reads
them" pattern is gone — constants are baked into bytecode.
File: packages/chain/src/Writer.sol:20-22, packages/chain/src/ColorRegistry.sol:7-8
bytes public DOMAIN_NAME = "Writer";
bytes public DOMAIN_VERSION = "1";
These are storage variables (not constant or immutable), so a future contract version that adds a setter could silently break
verification. They also depend on Solidity initializer ordering for the parent constructor to receive non-zero values (works today but
fragile). Make them string constant and pass literals to the parent constructor.
Also: bytes32 public WRITER_ROLE = keccak256("WRITER"); is a storage variable for what should be a constant. Use bytes32 public constant WRITER_ROLE = keccak256("WRITER");.
✅ L-2. EIP-712 domain name is bytes not string
ℹ️: Fixed during the L-1/L-2/M-9 cleanup. VerifyTypedData's constructor
now takes string memory name, string memory version (per EIP-712
spec) and uses keccak256(bytes(name)) internally. ABIs in
packages/utils/abis were updated to reflect the string return type.
File: packages/chain/src/Writer.sol:20, VerifyTypedData.sol:11-16
The EIP-712 spec says the name field is a string. You declare it as bytes and hash it. This works today because
keccak256(bytes("Writer")) == keccak256("Writer"), but it's nonstandard and may confuse off-chain tooling that introspects ABIs.
L-3. replaceAdmin revokes the wrong role if already-granted
File: packages/chain/src/Writer.sol:74-77
If the new admin is also the current admin (or vice versa via misconfiguration during deployment), _revokeRole(DEFAULT_ADMIN_ROLE,
msg.sender) will revoke the only admin. Two-step pattern (M-4) fixes this implicitly.
❌ L-4. WriterFactory doesn't validate inputs ℹ️: Won't fix. The "address(0) admin locks the contract" property is deliberately preserved as a feature for "written in stone" writers where no admin functions can ever be called (no setTitle, no setStorage, no replaceAdmin). Combined with publicWritable=true and a one-shot deploy, this lets you create a permanent, no-admin message board where the contract config can never be mutated. The contract-side input validation would foreclose this use case. The managers length is implicitly capped by the server endpoint via L-18 (and direct calls bypass the server but that's acceptable — advanced users who deploy directly accept the consequences).
File: packages/chain/src/WriterFactory.sol:17
create() accepts arbitrary title (no length cap), arbitrary admin (could be address(0), which then locks the contract), and an unbounded
managers array. The server already enforces MAX_TITLE_LENGTH, but the contract has no defense if called directly. Add require(admin !=
address(0)) and a sane managers length cap.
L-5. Factory salt collision DoS
File: packages/chain/src/WriterFactory.sol:25-38
If two callers pass the same salt the second create2 reverts. Server picks a random 32-byte salt so this is fine in practice, but if you ever expose salt to user input you'd want to mix in msg.sender.
✅ L-6. requireSavedAuth middleware ordering ℹ️: Fixed by wrapping the getAddress() normalization in try/catch inside both requireSavedAuth and requireWriterAdminAuth. Malformed URL params now return a clean 401 instead of bubbling up as a 500. The middleware is still ordered before the zod validator (matching the existing route registration), but it no longer crashes on bad input.
File: packages/server/src/routes/saved.ts:21-23,42, packages/server/src/privy.ts:41-48
requireSavedAuth runs before the zod param validator and reads c.req.param("userAddress") raw. If the user passes a non-address string in
the URL, getAddress(walletAddress) !== getAddress(userAddress) throws, which Hono returns as a 500 instead of a clean 400. Either run
validation first (note the schema is registered after the middleware in saved.ts) or wrap in a try/catch in privy.ts and return 401/403
explicitly.
❌ L-7. requireWriterAdminAuth reads from the DB, not from the chain ℹ️: Won't fix. Reading admin from chain on every /hide call would add a synchronous RPC roundtrip to a hot path, which is too slow. replaceAdmin is rare in practice (intended for emergency rotation, not routine ops), and the worst case after a rotation is a brief window (seconds, until the indexer catches up) where the old admin can still hide a writer they no longer own. Acceptable.
File: packages/server/src/privy.ts:79, packages/server/src/routes/writer.ts:218 (/writer/:address/hide)
If the on-chain admin is rotated via replaceAdmin, your DB still has the old admin until reindexing, so the old admin can still hide the writer. For most flows this is acceptable, but worth knowing. Stronger version: read admin from the chain via a contract call.
❌ L-8. /writer/:address/hide is an off-chain soft-delete ℹ️: Won't fix. The current behavior matches the project's mental model: "hide" removes the writer from your dashboard view, the chain stays forever. Users who care can verify on chain. Adding a tooltip is deferred — the existing UX is acceptable.
File: packages/server/src/routes/writer.ts:218-240
The endpoint is named "hide" and only flips a DB flag. Document this clearly to users — there is no on-chain "hide", and anyone reading
the chain or your /writer/public indexer logs can still find the writer. This is not a privacy feature.
❌ L-9. Soft-delete claim in CLAUDE.md is incorrect
ℹ️: delete onchain but visible via block history forever
CLAUDE.md says "Soft Deletes: Maintains blockchain history integrity." Actually, WriterStorage.remove (WriterStorage.sol:79-103) does a
hard delete of the entry struct and removes it from entryIds. Only events preserve history. Either fix the docs or implement a deletedAt
flag in the struct.
✅ L-10. ColorRegistry has the same malleability replay weakness as Writer
ℹ️: Fixed alongside C-2. ColorRegistry.setHexWithSig now uses the
digest-keyed digestWasExecuted map and OZ ECDSA's low-S enforcement
via the shared VerifyTypedData._recover helper. Same fix, same
guarantees as Writer.
Already covered by C-2; lower impact (only changes a user's color preference).
❌ L-11. getEntryContent is unbounded gas
ℹ️: simple helper generally not used
Looping string concatenation over an arbitrary number of chunks. View functions have no gas cap when called via eth_call, but the
frontend never calls this anyway. Recommend marking it external and only used as a debug helper, or removing it entirely.
❌ L-12. No pause/emergency stop
ℹ️: you can't pause rocks
Writer and WriterStorage have no pause() mechanism. If a critical bug is found post-launch you have no way to halt writes other than
removing all WRITER_ROLE grants from every Writer manually. Consider adding Pausable to Writer (with the admin as pauser) — at the cost
of a minor centralization tradeoff.
L-13. Reconcile / poller paths trust receipt logs by topic without verifying source
File: packages/server/src/helpers.ts:462-499, :658-660
You filter EntryCreated logs by log.address.toLowerCase() === entry.storageAddress.toLowerCase() — good. But for the writer
reconciliation you only check the factory address. If the relay submits a tx that calls a contract emitting a fake WriterCreated-shaped
log from FACTORY_ADDRESS (not actually possible because only the factory itself can emit that topic from that address), you'd be safe.
Just confirming the check is correct here. ✓
✅ L-14. delete entry db error paths log + leak DB error messages ℹ️: Fixed across all 10 sites in routes/writer.ts (7) and routes/saved.ts (3). Each handler now returns a generic operation-name string in the response (e.g. "database error during entry creation") and keeps the full error in the logs via console.error(err). No DB schema details, constraint names, or connection info leak to clients on a 500.
Files: all .post(...) handlers in routes/writer.ts
Errors are concatenated into the response: database error during entry creation: ${message}. In production you should log the message but return a generic string to the client to avoid leaking schema/connection info on a misconfiguration.
L-15. cors() is wide-open
File: packages/server/src/server.ts:11
app.use("*", cors()) defaults to Access-Control-Allow-Origin: *. Since Privy uses bearer tokens (not cookies) and you have no other
origin-bound auth, this is currently safe. If you ever add cookie-based session auth, lock CORS down to the writer.place origin.
L-16. signaturesWasExecuted map grows unbounded
Each successful sig call writes a permanent storage slot keyed off the signature (or, post-fix, the digest). Storage cost is on Optimism and reasonable, but it does mean per-Writer state grows linearly with operations forever. Acceptable; just note it.
❌ L-17. addChunk index check is signedByAuthorWithRole but author is checked via store.getEntry(id).author ℹ️: Won't fix. WriterStorage is intentionally NOT being modified in this redeploy so existing entries / WriterStorage instances remain binary-compatible (the C-2 migration via setLogic preserves storage state, and any cleanup change to WriterStorage would break that property). The implicit existence check via getEntry().author == signer is functionally safe — only cosmetic clarity is lost.
If id doesn't exist, getEntry returns a default struct with author = address(0), and the modifier requires signer == address(0), which
ecrecover only returns on failure. So calling for a nonexistent id reverts safely. ✓ Worth adding an explicit
require(_doesEntryExist(id)) for clarity.
✅ L-18. No zod validation on the /factory/create managers array length or admin/manager equality ℹ️: Fixed by changing the factoryCreateJsonValidator zod schema to require managers.length === 1. The UI flow always passes [callerWallet] as the managers list (the caller is also the admin), so requiring exactly 1 matches actual usage. Combined with the H-2 fix (admin === c.var.walletAddress), the only valid call shape via the server is "create a writer where I am both admin and sole manager." Direct contract callers (cast send to factory.create) bypass this and can pass any managers list — accepted (they're out of scope for the rate-limited / auth-gated server flow).
managers is unbounded; an attacker (after H-2 is fixed) with a Privy account could still attempt to deploy a Writer with thousands of
managers, blowing past the block gas limit. Add a sane max (e.g., 50).
What I did NOT cover
- The Drizzle schema, migrations, and any SQL injection surface (none expected with Drizzle, but not verified).
- The Ponder indexer.
- The Next.js frontend's XSS surface — particularly any place entry.raw or decrypted plaintext is rendered without sanitization. You
should review the TipTap config and any dangerouslySetInnerHTML paths separately. A user storingin a
public entry could XSS readers if rendering isn't escaped. - The Privy server-auth integration in detail (the SDK looks correctly used).
- The Syndicate → durable wallet relay implementation itself.
- Foundry tests for negative cases (you have positive tests; no tests for the malleability replay or the setStorage swap).
- Any deployment scripts / multisig hygiene for the factory and existing writers.
Recommended pre-launch order of operations
- C-1, C-2, H-1: redesign encryption key derivation (EIP-712 + HKDF + per-writer key + AAD), and add low-S enforcement plus digest-keyed replay in the contract. These are protocol-level and hardest to roll back. Plan a migration path for v2/v3 entries.
- H-2, H-3: lock down all relayed write endpoints behind Privy auth + recovered-signer match + rate limits. This is a small, mechanical PR that prevents relay drain.
- H-4: gate /manager/:address and /relay/wallets.
- M-3 / M-4 / M-6 / M-7 / L-1 / L-4: contract cleanup pass before mainnet deploy. Worth a fresh foundry test for malleability and admin edge cases.
- Frontend XSS review (out of scope above but mandatory before launch).
- Add an emergency Pausable (L-12) and document the admin trust model (M-3, L-8).
The most urgent items to fix today are C-1 (your privacy guarantee is currently breakable by any phishing site) and C-2 + H-3 (your relay funds are currently drainable by anyone). Everything else can be sequenced after.
⏺ Audit complete. The two issues that should block launch are:
- Encryption key derivation is phishable (signer.ts:209-279) — personal_sign of a fixed string has no domain binding, so any site can
recover the AES key. Move to EIP-712 + bind to writer address. - Signature malleability bypasses replay protection (Writer.sol:50-54, VerifyTypedData.sol:32-52) — _validateAndMarkSignature keys off
keccak256(signature), but ECDSA (r, n−s, v') is also valid and has a different hash. Enforce low-S and key the replay map off the digest, not the signature bytes.
Plus a serious operational risk: every relay-paying endpoint is unauthenticated (/factory/create, createWithChunk, update, delete,
color-registry/set). An attacker can drain your relay wallet without ever owning a Writer.
Full report above covers 2 critical, 4 high, 9 medium, and 18 low/info findings, with file:line references and concrete fixes.