From d861c0ad99d61b45667e5ba38d4d2bf735607f90 Mon Sep 17 00:00:00 2001 From: Jeff Emmett Date: Mon, 9 Mar 2026 17:17:50 -0700 Subject: [PATCH] fix(encryptid): harden wallet link flow + add device_registration type - Atomic nonce consumption prevents TOCTOU races - SIWE domain validation against allowlist - Unique constraint on linked_wallets(user_id, address_hash) - Add device_registration to challenge type enum Co-Authored-By: Claude Opus 4.6 --- modules/rwallet/mod.ts | 3 +++ src/encryptid/db.ts | 26 +++++++++++++++++++++++++- src/encryptid/schema.sql | 11 +++++++++-- src/encryptid/server.ts | 28 ++++++++++++---------------- 4 files changed, 49 insertions(+), 19 deletions(-) diff --git a/modules/rwallet/mod.ts b/modules/rwallet/mod.ts index 95b5a22..dfa17f8 100644 --- a/modules/rwallet/mod.ts +++ b/modules/rwallet/mod.ts @@ -431,6 +431,9 @@ routes.post("/api/safe/:chainId/:address/add-owner-proposal", async (c) => { if (!newOwner || !signature || !sender) { return c.json({ error: "Missing required fields: newOwner, signature, sender" }, 400); } + if (!/^0x[0-9a-fA-F]{40}$/.test(newOwner)) { + return c.json({ error: "Invalid newOwner address" }, 400); + } // Encode addOwnerWithThreshold(address owner, uint256 _threshold) // Function selector: 0x0d582f13 diff --git a/src/encryptid/db.ts b/src/encryptid/db.ts index 31f39ca..725a6aa 100644 --- a/src/encryptid/db.ts +++ b/src/encryptid/db.ts @@ -179,7 +179,7 @@ export async function getChallenge(challenge: string): Promise { await sql`DELETE FROM challenges WHERE challenge = ${challenge}`; } +export async function consumeChallenge( + challenge: string, + userId: string, + expectedType: StoredChallenge['type'], +): Promise { + const rows = await sql` + DELETE FROM challenges + WHERE challenge = ${challenge} + AND user_id = ${userId} + AND type = ${expectedType} + AND expires_at > NOW() + RETURNING * + `; + if (rows.length === 0) return null; + const row = rows[0]; + return { + challenge: row.challenge, + userId: row.user_id || undefined, + type: row.type as StoredChallenge['type'], + createdAt: new Date(row.created_at).getTime(), + expiresAt: new Date(row.expires_at).getTime(), + }; +} + export async function cleanExpiredChallenges(): Promise { const result = await sql`DELETE FROM challenges WHERE expires_at < NOW()`; return result.count; diff --git a/src/encryptid/schema.sql b/src/encryptid/schema.sql index 1610de0..ee8fa49 100644 --- a/src/encryptid/schema.sql +++ b/src/encryptid/schema.sql @@ -39,7 +39,7 @@ CREATE INDEX IF NOT EXISTS idx_credentials_user_id ON credentials(user_id); CREATE TABLE IF NOT EXISTS challenges ( challenge TEXT PRIMARY KEY, user_id TEXT, - type TEXT NOT NULL CHECK (type IN ('registration', 'authentication', 'wallet_link')), + type TEXT NOT NULL CHECK (type IN ('registration', 'authentication', 'device_registration', 'wallet_link')), created_at TIMESTAMPTZ DEFAULT NOW(), expires_at TIMESTAMPTZ NOT NULL ); @@ -319,7 +319,7 @@ CREATE INDEX IF NOT EXISTS idx_identity_invites_client_id ON identity_invites(cl ALTER TABLE challenges DROP CONSTRAINT IF EXISTS challenges_type_check; ALTER TABLE challenges ADD CONSTRAINT challenges_type_check - CHECK (type IN ('registration', 'authentication', 'wallet_link')); + CHECK (type IN ('registration', 'authentication', 'device_registration', 'wallet_link')); -- ============================================================================ -- LINKED EXTERNAL WALLETS (SIWE-verified wallet associations) @@ -341,3 +341,10 @@ CREATE TABLE IF NOT EXISTS linked_wallets ( CREATE INDEX IF NOT EXISTS idx_linked_wallets_user_id ON linked_wallets(user_id); CREATE INDEX IF NOT EXISTS idx_linked_wallets_address_hash ON linked_wallets(address_hash); + +-- Prevent duplicate wallet links per user (application-level check + DB enforcement) +DO $$ BEGIN + ALTER TABLE linked_wallets ADD CONSTRAINT linked_wallets_user_address_unique + UNIQUE (user_id, address_hash); +EXCEPTION WHEN duplicate_table THEN NULL; +END $$; diff --git a/src/encryptid/server.ts b/src/encryptid/server.ts index b2ab726..386f4de 100644 --- a/src/encryptid/server.ts +++ b/src/encryptid/server.ts @@ -100,6 +100,7 @@ import { getLinkedWallets, deleteLinkedWallet, linkedWalletExists, + consumeChallenge, sql, } from './db.js'; import { @@ -2754,30 +2755,28 @@ app.post('/encryptid/api/wallet-link/verify', async (c) => { return c.json({ error: 'Invalid SIWE message format' }, 400); } - // Validate nonce exists and belongs to this user - const challenge = await getChallenge(parsed.nonce); + // Atomically consume the nonce (DELETE...RETURNING prevents TOCTOU races) + const challenge = await consumeChallenge(parsed.nonce, claims.sub, 'wallet_link'); if (!challenge) { - return c.json({ error: 'Invalid or expired nonce' }, 400); - } - if (challenge.userId !== claims.sub) { - return c.json({ error: 'Nonce does not belong to this user' }, 403); - } - if (Date.now() > challenge.expiresAt) { - await deleteChallenge(parsed.nonce); - return c.json({ error: 'Nonce expired' }, 400); + return c.json({ error: 'Invalid, expired, or already-used nonce' }, 400); } - // Validate SIWE message fields + // Validate SIWE message fields against server-known domains + const allowedDomains = [CONFIG.rpId || 'rspace.online', 'rwallet.online']; + const messageDomain = parsed.domain || ''; + if (!allowedDomains.some(d => messageDomain === d || messageDomain.endsWith(`.${d}`))) { + return c.json({ error: 'SIWE domain not recognized' }, 400); + } const isValid = validateSiweMessage({ message: parsed, - domain: parsed.domain || '', + domain: messageDomain, nonce: parsed.nonce, }); if (!isValid) { return c.json({ error: 'SIWE message validation failed' }, 400); } - // Verify the signature + // Verify the signature cryptographically const recovered = await verifyMessage({ address: parsed.address as `0x${string}`, message, @@ -2787,9 +2786,6 @@ app.post('/encryptid/api/wallet-link/verify', async (c) => { return c.json({ error: 'Signature verification failed' }, 400); } - // Clean up nonce - await deleteChallenge(parsed.nonce); - // Check for duplicate const exists = await linkedWalletExists(claims.sub, addressHash); if (exists) {