diff --git a/src/servers/sidecar/wallet/backends/onchain.ts b/src/servers/sidecar/wallet/backends/onchain.ts index 0463a3eb..69a26f89 100644 --- a/src/servers/sidecar/wallet/backends/onchain.ts +++ b/src/servers/sidecar/wallet/backends/onchain.ts @@ -719,6 +719,17 @@ export class OnchainBackend extends BaseBackend { // Anything past the scanned window is unused by definition — the gap limit is what ended the scan. while (index <= highest && used.has(index)) index++; + // ...but "unused" is not the same as "findable". The mark advances on every issuance, paid or not, + // so twenty unpaid addresses walk it past the end of the window the next scan covers, and a payment + // to the twenty-first would never be seen again. Rather than hand out an address the wallet could + // lose money on, stop and re-offer the last safe one: every index in that run is still virgin, so + // repeating one is not address reuse in the sense that matters — nothing was ever received on it. + // The floor of 0 covers a wallet the chain has never seen: Esplora short-circuits an account with no + // history and returns no addresses at all, leaving `highest` at -1, and index 0 is then the only one + // it will look at. Handing that out until it is paid is exactly right. + const ceiling = Math.max(0, highest + this.chain.issueAhead); + if (index > ceiling) index = ceiling; + if (!peek) this.issued.set(key, index + 1); return this.derive(type, chain, index); } diff --git a/src/servers/sidecar/wallet/chain-source-esplora.test.ts b/src/servers/sidecar/wallet/chain-source-esplora.test.ts index c0b6cd12..d060f245 100644 --- a/src/servers/sidecar/wallet/chain-source-esplora.test.ts +++ b/src/servers/sidecar/wallet/chain-source-esplora.test.ts @@ -248,6 +248,26 @@ describe('EsploraChainSource through OnchainBackend', () => { expect((await backend.getNewAddress()).address).toBe(addressAt(0, 2)); }); + // Issuance used to be unbounded: the mark moved on every call, paid or not, so a run of unpaid + // addresses walked it past the end of the window the next scan covers. A payment there would have been + // invisible forever — Esplora has no rescan to go and look. + test('issuance stops at the far edge of the scanned window instead of walking past it', async () => { + const world = new Map([[addressAt(0, 0), { confirmed: 5_000 }]]); + const { backend } = backendFor(world); + + const issued: string[] = []; + for (let i = 0; i < 30; i++) issued.push((await backend.getNewAddress()).address); + + // 0/0 is used, so the walk covers 0..20 — one full gap limit of misses past it — and stops. + expect(issued[0]).toBe(addressAt(0, 1)); + expect(issued[19]).toBe(addressAt(0, 20)); + // Everything after that re-offers the last address the scan can still see. All of them are virgin, + // so repeating one is not address reuse in the sense that loses privacy. + expect(issued.at(-1)).toBe(addressAt(0, 20)); + const scanned = new Set(Array.from({ length: 21 }, (_, i) => addressAt(0, i))); + expect(issued.filter((a) => !scanned.has(a))).toEqual([]); + }); + test('transactions are scored against the wallet and credit is positive', async () => { const world = new Map([[addressAt(0, 0), { confirmed: 50_000 }]]); const { backend } = backendFor(world); diff --git a/src/servers/sidecar/wallet/chain-source-esplora.ts b/src/servers/sidecar/wallet/chain-source-esplora.ts index bf4c7d0c..88d3a91e 100644 --- a/src/servers/sidecar/wallet/chain-source-esplora.ts +++ b/src/servers/sidecar/wallet/chain-source-esplora.ts @@ -37,6 +37,10 @@ export type EsploraChainSourceOptions = { chain: EsploraChain; label: string }; export class EsploraChainSource implements WalletChainSource { readonly label: string; + + /** The walk already runs a full GAP_LIMIT past the last used address, and stops dead there. */ + readonly issueAhead = 0; + private readonly chain: EsploraChain; constructor(opts: EsploraChainSourceOptions) { diff --git a/src/servers/sidecar/wallet/chain-source-nbxplorer.ts b/src/servers/sidecar/wallet/chain-source-nbxplorer.ts index 4956ca20..2c61755b 100644 --- a/src/servers/sidecar/wallet/chain-source-nbxplorer.ts +++ b/src/servers/sidecar/wallet/chain-source-nbxplorer.ts @@ -57,6 +57,16 @@ export type NbxplorerChainSourceOptions = { chain: NbxplorerChain; network: Bitc export class NbxplorerChainSource implements WalletChainSource { readonly label: string; + + /** + * A scan reports up to NBXplorer's unused mark, but NBXplorer itself watches a gap beyond it + * (`MinGapSize`, 20 by default) and extends as addresses are used, so an address a little past the + * mark is still seen. Deliberately below that default rather than equal to it, because the setting + * belongs to the node operator and is not readable from here — and unlike Esplora, a miss on this + * source is recoverable: `utxos/scan` walks the node's whole UTXO set to a gap limit of 1000. + */ + readonly issueAhead = 10; + private readonly chain: NbxplorerChain; private readonly btcNetwork: bitcoin.Network; diff --git a/src/servers/sidecar/wallet/chain-source.ts b/src/servers/sidecar/wallet/chain-source.ts index 2f74538e..2667ad07 100644 --- a/src/servers/sidecar/wallet/chain-source.ts +++ b/src/servers/sidecar/wallet/chain-source.ts @@ -96,6 +96,21 @@ export interface WalletChainSource { /** Human-readable, for logs and the sync-state surface. e.g. `esplora(mempool.space)`. */ readonly label: string; + /** + * How many indices past the highest one `scan` reported this source will still find a payment on. + * + * Issuance must not outrun the search that finds the money again. `nextUnused` advances a mark every + * time an address is handed out, so a run of addresses issued and never paid walks the mark forward + * with nothing marking those indices used — and a later payment to one of them lands beyond the + * window the next scan covers. On a source that cannot rescan, that is an invisible, unrecoverable + * balance, from nothing worse than clicking "new address" too many times. + * + * Esplora is 0: its walk already extends a full gap limit past the last used address, so every index + * `scan` returned is covered and none beyond it is. NBXplorer reports only up to its own unused mark + * but watches a gap beyond it, so a modest overrun is still seen. + */ + readonly issueAhead: number; + /** Current best block height. The cheapest liveness probe the wallet has. */ getTipHeight(): Promise; diff --git a/src/servers/sidecar/wallet/keys.test.ts b/src/servers/sidecar/wallet/keys.test.ts index b122a118..96ec2d57 100644 --- a/src/servers/sidecar/wallet/keys.test.ts +++ b/src/servers/sidecar/wallet/keys.test.ts @@ -95,13 +95,40 @@ describe('sealSeed / deriveAccountXpubs', () => { ); }); +/** + * A throwaway wallet id per call. Every passphrase check is now rate-limited per wallet id in module + * state, so tests that share one would start locking each other out after five wrong guesses. + */ +let idSeq = 0; +const wid = () => ++idSeq; + describe('passphrase handling', () => { test( 'accepts the right passphrase and rejects a wrong one', async () => { const env = await sealSeed(VECTOR, PASS); - expect(await verifyPassphrase(env, PASS)).toBe(true); - expect(await verifyPassphrase(env, 'not the passphrase')).toBe(false); + expect(await verifyPassphrase(wid(), env, PASS)).toBe(true); + expect(await verifyPassphrase(wid(), env, 'not the passphrase')).toBe(false); + }, + SLOW, + ); + + test( + 'locks out after five wrong guesses, on export as well as unlock', + async () => { + const env = await sealSeed(VECTOR, PASS); + const id = wid(); + + // The backoff used to live inside UnlockSession.unlock only, so export-seed — which returns the + // words in the clear — took unlimited guesses. Exercised through exportMnemonic for that reason. + for (let i = 0; i < 5; i++) { + await expect(exportMnemonic(id, env, `wrong ${i}`)).rejects.toThrow(/incorrect passphrase/); + } + await expect(exportMnemonic(id, env, `wrong 5`)).rejects.toThrow(/too many failed attempts/); + // The right passphrase is refused too — otherwise the lockout would only slow a guesser down. + await expect(exportMnemonic(id, env, PASS)).rejects.toThrow(/too many failed attempts/); + // And it is not folded into a bland `false` by the boolean-returning sibling. + await expect(verifyPassphrase(id, env, PASS)).rejects.toThrow(/too many failed attempts/); }, SLOW, ); @@ -110,7 +137,7 @@ describe('passphrase handling', () => { 'round-trips the mnemonic exactly', async () => { const env = await sealSeed(VECTOR, PASS); - expect(await exportMnemonic(env, PASS)).toBe(VECTOR); + expect(await exportMnemonic(wid(), env, PASS)).toBe(VECTOR); }, SLOW, ); @@ -120,11 +147,11 @@ describe('passphrase handling', () => { async () => { const env = await sealSeed(VECTOR, PASS); const next = 'an entirely different passphrase'; - const rotated = await changePassphrase(env, PASS, next); + const rotated = await changePassphrase(wid(), env, PASS, next); - expect(await verifyPassphrase(rotated, next)).toBe(true); - expect(await verifyPassphrase(rotated, PASS)).toBe(false); - expect(await exportMnemonic(rotated, next)).toBe(VECTOR); + expect(await verifyPassphrase(wid(), rotated, next)).toBe(true); + expect(await verifyPassphrase(wid(), rotated, PASS)).toBe(false); + expect(await exportMnemonic(wid(), rotated, next)).toBe(VECTOR); // The xpubs are stored alongside the envelope; if rotation changed them the wallet would silently // start watching a different account and report a zero balance. @@ -226,7 +253,7 @@ describe('aezeed seeds', () => { 'the words survive an export round trip', async () => { const env = await sealSeed(AEZEED, PASS); - expect(await exportMnemonic(env, PASS)).toBe(AEZEED); + expect(await exportMnemonic(wid(), env, PASS)).toBe(AEZEED); }, SLOW, ); @@ -270,7 +297,7 @@ describe('aezeed seeds', () => { async () => { const env = await sealSeed(AEZEED, PASS); const next = 'an entirely different passphrase'; - const rotated = await changePassphrase(env, PASS, next); + const rotated = await changePassphrase(wid(), env, PASS, next); expect(rotated.v).toBe(2); const after = await deriveAccountXpubs(rotated, next, 'bitcoin'); @@ -289,7 +316,7 @@ describe('generateSeed', () => { expect(generateSeed()).not.toBe(mnemonic); const env = await sealSeed(mnemonic, 'a sufficiently long passphrase'); - expect(await exportMnemonic(env, 'a sufficiently long passphrase')).toBe(mnemonic); + expect(await exportMnemonic(wid(), env, 'a sufficiently long passphrase')).toBe(mnemonic); }, SLOW, ); diff --git a/src/servers/sidecar/wallet/keys.ts b/src/servers/sidecar/wallet/keys.ts index cae80d3d..5c670f33 100644 --- a/src/servers/sidecar/wallet/keys.ts +++ b/src/servers/sidecar/wallet/keys.ts @@ -352,15 +352,7 @@ export class UnlockSession { } async unlock(env: SeedEnvelope, ownerPassphrase: string, ttlSec = DEFAULT_TTL_SEC): Promise { - checkLockout(this.walletId); - let opened: OpenedSeed; - try { - opened = await openEnvelope(env, ownerPassphrase); - } catch (err) { - recordFailure(this.walletId); - throw err; - } - attempts.delete(this.walletId); + const opened = await openGuarded(this.walletId, env, ownerPassphrase); this.lock(); // replace any existing session rather than leaking the old root this.root = await rootFromSeed(opened); @@ -421,15 +413,40 @@ export function lockAll(): void { for (const s of sessions.values()) s.lock(); } +/** + * Open an envelope under the same backoff `unlock()` uses. + * + * EVERY passphrase check goes through here, not just the session one. The lockout used to live inside + * UnlockSession.unlock alone, which left export-seed — the one endpoint that returns the words in the + * clear — accepting unlimited guesses, while /unlock capped at five a minute. An attacker with a session + * cookie would simply never have used /unlock. + */ +async function openGuarded(walletId: number, env: SeedEnvelope, ownerPassphrase: string): Promise { + checkLockout(walletId); + let opened: OpenedSeed; + try { + opened = await openEnvelope(env, ownerPassphrase); + } catch (err) { + recordFailure(walletId); + throw err; + } + attempts.delete(walletId); + return opened; +} + /** * Verify a passphrase without opening a session — used before destructive operations (seed export, * wallet deletion) so they need a fresh confirmation even when the wallet is already unlocked. + * + * Rethrows a LOCKED_OUT BackendError rather than folding it into `false`: "wrong passphrase" and "stop + * guessing" are different answers, and a caller that showed the first for both would loop forever. */ -export async function verifyPassphrase(env: SeedEnvelope, ownerPassphrase: string): Promise { +export async function verifyPassphrase(walletId: number, env: SeedEnvelope, ownerPassphrase: string): Promise { try { - await openEnvelope(env, ownerPassphrase); + await openGuarded(walletId, env, ownerPassphrase); return true; - } catch { + } catch (err) { + if (err instanceof BackendError && err.code === 'LOCKED_OUT') throw err; return false; } } @@ -439,8 +456,8 @@ export async function verifyPassphrase(env: SeedEnvelope, ownerPassphrase: strin * can back up or migrate. Always requires the passphrase even if a session is open, and callers must * gate it behind a fresh confirmation. */ -export async function exportMnemonic(env: SeedEnvelope, ownerPassphrase: string): Promise { - const opened = await openEnvelope(env, ownerPassphrase); +export async function exportMnemonic(walletId: number, env: SeedEnvelope, ownerPassphrase: string): Promise { + const opened = await openGuarded(walletId, env, ownerPassphrase); return opened.mnemonic; } @@ -450,11 +467,12 @@ export async function exportMnemonic(env: SeedEnvelope, ownerPassphrase: string) * old passphrase, cannot decrypt anything written after a rotation. */ export async function changePassphrase( + walletId: number, env: SeedEnvelope, oldPassphrase: string, newPassphrase: string, ): Promise { - const opened = await openEnvelope(env, oldPassphrase); + const opened = await openGuarded(walletId, env, oldPassphrase); return sealSeed(opened.mnemonic, newPassphrase, opened.bip39Passphrase || undefined); } diff --git a/src/servers/sidecar/wallet/routes.ts b/src/servers/sidecar/wallet/routes.ts index 4780b7fb..f775db1b 100644 --- a/src/servers/sidecar/wallet/routes.ts +++ b/src/servers/sidecar/wallet/routes.ts @@ -491,7 +491,7 @@ async function deleteWalletRoute(ctx: OfficerContext, walletId: number): Promise if (!passphrase) return badRequest('passphrase is required to delete a seeded wallet'); const sealed = await getSealedSeed(ctx.userId, walletId); if (!sealed) throw new BackendError('wallet seed is missing', 500, 'NO_SEED'); - if (!(await verifyPassphrase(JSON.parse(sealed) as SeedEnvelope, passphrase))) { + if (!(await verifyPassphrase(walletId, JSON.parse(sealed) as SeedEnvelope, passphrase))) { return json({ error: 'incorrect passphrase' }, 401); } } @@ -530,7 +530,7 @@ async function changePassphraseRoute(ctx: OfficerContext, walletId: number): Pro if (!oldPassphrase || !newPassphrase) return badRequest('oldPassphrase and newPassphrase are required'); const env = await loadEnvelope(ctx.userId, walletId); - const resealed = await changePassphrase(env, oldPassphrase, newPassphrase); + const resealed = await changePassphrase(walletId, env, oldPassphrase, newPassphrase); 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(); @@ -543,7 +543,7 @@ async function exportSeedRoute(ctx: OfficerContext, walletId: number): Promise