diff --git a/packages/signer/src/policy.ts b/packages/signer/src/policy.ts index 948f2ef..8dff899 100644 --- a/packages/signer/src/policy.ts +++ b/packages/signer/src/policy.ts @@ -24,14 +24,27 @@ function ada(lovelace: bigint): string { return (Number(lovelace) / 1_000_000).toFixed(6); } -// A blank entry is not a usable address. `''.split(',')` on an empty string -// yields `['']`, not `[]` — a realistic way an upstream config/env-parsing bug -// could hand this module a non-empty array that names no real address. Left -// unfiltered, `ownAddresses.length === 0` would miss that case (the array -// isn't empty), the blank entry would never match a real output, and the -// module would proceed as if it knew an own address when it does not. +// A blank entry is not a usable address — and neither is one built entirely +// from characters that merely look blank. `''.split(',')` on an empty string +// yields `['']`, not `[]`, a realistic way an upstream config/env-parsing bug +// could hand this module a non-empty array that names no real address; a +// literal '' is the obvious case, but `.trim()` alone does not generalise +// past it, since it only strips the ECMAScript WhiteSpace set (which +// includes NBSP U+00A0 and BOM U+FEFF). Plenty of other invisible characters +// survive it: U+200B ZERO WIDTH SPACE and U+2060 WORD JOINER are Unicode +// category Cf (format), not Zs (space separator), so `.trim()` leaves them +// untouched. A denylist of "invisible" codepoints is not a fix — there are +// dozens of Cf/Cc codepoints and more get added — so this checks positively +// instead: an entry only counts as a known own address if, after trimming, +// it starts with the 'addr' prefix every Cardano/Vector payment address +// actually uses. Nothing built purely from blank or invisible characters can +// satisfy that, without needing to enumerate any of them. +function looksLikeAddress(address: string): boolean { + return address.trim().startsWith('addr'); +} + function knownAddresses(ownAddresses: string[]): string[] { - return ownAddresses.filter((address) => address.trim().length > 0); + return ownAddresses.filter(looksLikeAddress); } /** @@ -60,6 +73,22 @@ export function computeOutflow( return { netOutflowLovelace: leaving + tx.fee, assetMovements }; } +/** + * Decides whether to sign `tx`, bounding lovelace outflow only. + * + * `SpendLimits` has no asset-value dimension: `assetMovements` (native + * assets leaving to a foreign address, e.g. NFTs or token balances) is + * computed and reported on the returned `PolicyDecision` for visibility, but + * it never gates `allowed`. A transaction can move an entire native-asset + * balance out of the wallet and still receive `allowed: true` as long as the + * lovelace side of that same transaction stays under both limits — e.g. an + * NFT sent with only the minimum UTxO deposit attached. This is a deliberate + * v1 scope boundary, not an oversight: adding asset-value limits is future + * work, and would need `SpendLimits` itself to grow a corresponding field. + * A caller must not read `allowed: true` from this function as approval of + * whatever `assetMovements` contains — only as approval of the lovelace + * amount in `netOutflowLovelace`. + */ export function evaluate( tx: DecodedTx, ownAddresses: string[], diff --git a/packages/signer/test/policy.test.ts b/packages/signer/test/policy.test.ts index f246945..bfb4d69 100644 --- a/packages/signer/test/policy.test.ts +++ b/packages/signer/test/policy.test.ts @@ -167,6 +167,94 @@ describe('evaluate — ownAddresses containing only blank entries', () => { }); }); +describe('evaluate — ownAddresses entries built from invisible Unicode characters', () => { + // `.trim()` alone is not enough to detect "not a real address": it strips + // the ECMAScript WhiteSpace set (which includes NBSP U+00A0 and BOM + // U+FEFF) but not every invisible character. U+200B ZERO WIDTH SPACE and + // U+2060 WORD JOINER are Unicode category Cf (format), not Zs (space + // separator) or part of ECMAScript's WhiteSpace production, so trimming + // either one leaves a non-empty string — a trim-then-check-length filter + // alone would let either slip past exactly the way '' used to. (U+00A0 and + // U+FEFF, by contrast, ARE stripped by .trim() and were never actually a + // gap here — verified directly before writing these tests: only U+200B and + // U+2060 reproduce it.) A denylist of "invisible" codepoints is not the + // fix: there are dozens of Cf/Cc codepoints and more get added, so this is + // a positive check instead — an entry only counts as a known own address + // if, after trimming, it starts with the 'addr' prefix every + // Cardano/Vector payment address actually uses. Nothing built purely from + // invisible characters can satisfy that. + // Built via fromCharCode rather than a literal escape so the codepoint is + // unambiguous in source and cannot be misrendered by an editor or diff tool. + const ZERO_WIDTH_SPACE = String.fromCharCode(0x200b); + const WORD_JOINER = String.fromCharCode(0x2060); + + test('a single ZERO WIDTH SPACE (U+200B) entry is treated as no known own address', () => { + const d = evaluate( + tx({ outputs: [{ address: FOREIGN, lovelace: 5_000_000n, assets: [] }] }), + [ZERO_WIDTH_SPACE], LIMITS, 0n + ); + assert.equal(d.allowed, false); + assert.match(d.reason!, /own address/i); + }); + + test('a single WORD JOINER (U+2060) entry is treated as no known own address', () => { + const d = evaluate( + tx({ outputs: [{ address: FOREIGN, lovelace: 5_000_000n, assets: [] }] }), + [WORD_JOINER], LIMITS, 0n + ); + assert.equal(d.allowed, false); + assert.match(d.reason!, /own address/i); + }); + + test('a genuine addr1... address is still accepted (the positive check does not over-reject)', () => { + const r = computeOutflow( + tx({ outputs: [{ address: OWN, lovelace: 5_000_000n, assets: [] }] }), + [OWN] + ); + assert.equal(r.netOutflowLovelace, 200_000n, 'a real own address must still be recognised as change'); + }); +}); + +describe('computeOutflow — the filter applies at the Set-construction site itself, not only in evaluate()', () => { + // evaluate()'s fail-closed guard and computeOutflow()'s own Set both call + // knownAddresses(), but nothing above exercises them independently: every + // prior blank/invisible-entry test either goes through evaluate() (whose + // guard would refuse before computeOutflow's classification could matter) + // or pairs a blank ownAddresses entry with a *real* output address (which + // would pass either way, filtered or not). That leaves a real gap: if a + // future edit filtered ownAddresses in evaluate()'s guard but left + // computeOutflow's `new Set(ownAddresses)` unfiltered, nothing here would + // catch it, because a blank entry in an otherwise-unfiltered Set only + // matters when something can actually match it — i.e. when an output + // address is *also* blank. decode.ts can never produce that: every output + // address comes from `o.address().to_bech32()`, which throws before an + // empty or blank address could ever reach a DecodedOutput (see decode.ts's + // per-output try/catch). So this is unreachable through the real pipeline + // — but computeOutflow is an exported pure function with its own contract, + // and that contract must hold regardless of what the current pipeline + // happens to be able to construct. + test('a blank output address is counted as outflow, not matched as change via a blank ownAddresses entry', () => { + const r = computeOutflow( + tx({ outputs: [{ address: '', lovelace: 5_000_000n, assets: [] }] }), + [''] + ); + assert.equal( + r.netOutflowLovelace, + 5_200_000n, + 'a blank output address must never be classified as change, even against a blank ownAddresses entry' + ); + }); + + test('an output address built from an invisible character is counted as outflow, not matched as change', () => { + const ZERO_WIDTH_SPACE = String.fromCharCode(0x200b); + const r = computeOutflow( + tx({ outputs: [{ address: ZERO_WIDTH_SPACE, lovelace: 5_000_000n, assets: [] }] }), + [ZERO_WIDTH_SPACE] + ); + assert.equal(r.netOutflowLovelace, 5_200_000n); + }); +}); + describe('evaluate — exact limit boundaries, one lovelace either side', () => { test('one lovelace under the per-transaction limit is allowed', () => { const d = evaluate(