rate-limit every passphrase check, and stop address issuance outrunning the scan
two holes found auditing the wallet sidecar after the frozen-utxo fix. the brute-force backoff lived inside UnlockSession.unlock alone, so /unlock capped at five guesses a minute while export-seed — the one endpoint that returns the words in the clear — took unlimited ones. every passphrase check now goes through the same guard. verifyPassphrase rethrows LOCKED_OUT rather than folding it into `false`, so a caller can tell "wrong" from "stop". nextUnused advanced its mark on every issuance, 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 never be found again, and esplora has no rescan to go looking. sources now declare how far past a scan's last index they can still see, and issuance clamps to it — re-offering a virgin address rather than handing out one that could lose money. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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<string, Funding>([[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<string, Funding>([[addressAt(0, 0), { confirmed: 50_000 }]]);
|
||||
const { backend } = backendFor(world);
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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<number>;
|
||||
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
|
||||
@@ -352,15 +352,7 @@ export class UnlockSession {
|
||||
}
|
||||
|
||||
async unlock(env: SeedEnvelope, ownerPassphrase: string, ttlSec = DEFAULT_TTL_SEC): Promise<void> {
|
||||
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<OpenedSeed> {
|
||||
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<boolean> {
|
||||
export async function verifyPassphrase(walletId: number, env: SeedEnvelope, ownerPassphrase: string): Promise<boolean> {
|
||||
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<string> {
|
||||
const opened = await openEnvelope(env, ownerPassphrase);
|
||||
export async function exportMnemonic(walletId: number, env: SeedEnvelope, ownerPassphrase: string): Promise<string> {
|
||||
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<SeedEnvelope> {
|
||||
const opened = await openEnvelope(env, oldPassphrase);
|
||||
const opened = await openGuarded(walletId, env, oldPassphrase);
|
||||
return sealSeed(opened.mnemonic, newPassphrase, opened.bip39Passphrase || undefined);
|
||||
}
|
||||
|
||||
|
||||
@@ -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<R
|
||||
if (!passphrase) return badRequest('passphrase is required');
|
||||
|
||||
const env = await loadEnvelope(ctx.userId, walletId);
|
||||
const mnemonic = await exportMnemonic(env, passphrase);
|
||||
const mnemonic = await exportMnemonic(walletId, env, passphrase);
|
||||
console.warn(`[wallet] seed exported for wallet ${walletId} by user ${ctx.userId}`);
|
||||
return json({ mnemonic, hasBip39Passphrase: env.hasBip39Passphrase });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user