From 1d9a648ff7f3831986c1e0ba84968c8eba72a9c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 4 Aug 2026 20:20:12 +0000 Subject: [PATCH] stop the generic wallet PATCH from being able to replace the seed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the route typed its body as {name, defaultBip, config} and passed the parsed object straight to updateWallet, which also accepted sealedSeed for the passphrase change. a TypeScript annotation strips nothing at runtime, so any authenticated caller could send a sealedSeed key and overwrite the encrypted seed — no passphrase, no unlock. encryptSecret encrypts nonsense happily, so the damage would have surfaced at the next unlock, not at the write. updateWallet can no longer touch the seed at all; resealing moves to replaceSealedSeed, whose only caller has already proved knowledge of the old passphrase. the route rebuilds its patch field by field as well, so the next field added there cannot re-open it. verified against the test wallet: the envelope is byte-identical after the same request that previously would have replaced it. Co-Authored-By: Claude Opus 5 --- src/databases/officer_db/src/index.ts | 1 + .../officer_db/src/queries/wallet.ts | 25 +++++++++++++++++-- src/servers/sidecar/wallet/routes.ts | 12 +++++++-- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/databases/officer_db/src/index.ts b/src/databases/officer_db/src/index.ts index 446f7c20..e05f4a1f 100644 --- a/src/databases/officer_db/src/index.ts +++ b/src/databases/officer_db/src/index.ts @@ -208,6 +208,7 @@ export { getSealedSeed, createWallet, updateWallet, + replaceSealedSeed, setActiveWallet, deleteWallet, getWalletLabels, diff --git a/src/databases/officer_db/src/queries/wallet.ts b/src/databases/officer_db/src/queries/wallet.ts index b5ae1139..cc8579aa 100644 --- a/src/databases/officer_db/src/queries/wallet.ts +++ b/src/databases/officer_db/src/queries/wallet.ts @@ -176,16 +176,19 @@ export async function createWallet(params: CreateWalletParams): Promise; defaultBip?: number; sealedSeed?: string }, + patch: { name?: string; config?: Record; defaultBip?: number }, ): Promise { const set: Record = { updatedAt: new Date() }; if (patch.name !== undefined) set.name = patch.name; if (patch.defaultBip !== undefined) set.defaultBip = patch.defaultBip; if (patch.config !== undefined) set.config = encryptSecret(JSON.stringify(patch.config)); - if (patch.sealedSeed !== undefined) set.seedEnvelope = encryptSecret(patch.sealedSeed); const [row] = await db .update(walletWallets) @@ -195,6 +198,24 @@ export async function updateWallet( return row ? toSummary(row) : null; } +/** + * Overwrite the encrypted seed. The ONLY caller is the passphrase change, which reseals the same words + * under a new passphrase and has already proved knowledge of the old one. + * + * It is a function of its own, rather than a fourth field on updateWallet, because it was one: the PATCH + * route typed its request body as `{name, defaultBip, config}` and passed the parsed object straight + * through, and a TypeScript annotation strips nothing at runtime. Any authenticated caller could send a + * `sealedSeed` key and replace the seed — no passphrase, no unlock, no confirmation — and since + * encryptSecret happily encrypts nonsense, the damage only surfaced at the next unlock attempt. Keeping + * the two apart means the next field added to that body cannot re-open it. + */ +export async function replaceSealedSeed(userId: number, id: number, sealedSeed: string): Promise { + await db + .update(walletWallets) + .set({ seedEnvelope: encryptSecret(sealedSeed), updatedAt: new Date() }) + .where(and(eq(walletWallets.userId, userId), eq(walletWallets.id, id))); +} + /** Exactly one active wallet per owner. Cleared and set in one transaction; the partial unique index is the backstop. */ export async function setActiveWallet(userId: number, id: number): Promise { await db.transaction(async (tx) => { diff --git a/src/servers/sidecar/wallet/routes.ts b/src/servers/sidecar/wallet/routes.ts index e1009b5b..6ad38a29 100644 --- a/src/servers/sidecar/wallet/routes.ts +++ b/src/servers/sidecar/wallet/routes.ts @@ -4,6 +4,7 @@ import { getSealedSeed, createWallet, updateWallet, + replaceSealedSeed, setActiveWallet, deleteWallet, getActiveWallet, @@ -173,7 +174,14 @@ async function handleWallets(ctx: OfficerContext, seg: string[]): Promise 64) return badRequest('name is too long (64 characters max)'); patch.name = name; } - const updated = await updateWallet(userId, walletId, patch); + // Rebuilt field by field rather than forwarded: `body` is a cast, so the parsed object holds + // whatever the caller sent, not what the type says. updateWallet can no longer write the seed + // either — that is the belt to this brace. + const updated = await updateWallet(userId, walletId, { + name: patch.name, + defaultBip: patch.defaultBip, + config: patch.config, + }); invalidate(walletId); return updated ? json({ wallet: updated }) : json({ error: 'wallet not found' }, 404); } @@ -520,7 +528,7 @@ async function changePassphraseRoute(ctx: OfficerContext, walletId: number): Pro const env = await loadEnvelope(ctx.userId, walletId); const resealed = await changePassphrase(env, oldPassphrase, newPassphrase); - await updateWallet(ctx.userId, walletId, { sealedSeed: JSON.stringify(resealed) }); + await replaceSealedSeed(ctx.userId, walletId, JSON.stringify(resealed)); // Force a re-unlock under the new passphrase rather than leaving a session opened by the old one. sessionFor(walletId).lock(); return json({ ok: true });