From 4a34066a1d0759bd0173a531341e6888b4e2a318 Mon Sep 17 00:00:00 2001 From: Aljaz Ceru Date: Mon, 17 Aug 2026 21:38:34 +0200 Subject: [PATCH] grantless: refuse silent amount raises, stricter bolt11 shape, payment-safe UI errors - resolveZapAmount: recipient minimum above the intended zap aborts with a clear message (hostile minSendable can no longer inflate an auto-paid NWC zap); only clamping DOWN is allowed; unit-tested - decodeBolt11: reject mixed-case bech32 and invoices without a payment_hash tag (unreceivable invoices no longer reach recovery QRs); crafted test fixtures now carry payment hashes; signature delegation to wallets documented (recovery-from-sig adds no integrity for no-n-tag invoices) - zap()/retry: post-payment UI failures can never report a paid zap as failed (double-pay guard); finishZap preference handling exception-safe - QR wording: bound invoices say payment 'should' publish the receipt via the recipient's server --- test/zap-core.test.js | 80 +++++++++++++++++++++++++++++++++++++++---- zap-core.js | 31 +++++++++++++++-- zap.js | 75 +++++++++++++++++++++++++++------------- 3 files changed, 152 insertions(+), 34 deletions(-) diff --git a/test/zap-core.test.js b/test/zap-core.test.js index 454d498..58e8963 100644 --- a/test/zap-core.test.js +++ b/test/zap-core.test.js @@ -21,22 +21,37 @@ function sha256Hex(s) { } // craft a structurally-valid BOLT11 invoice (valid bech32 checksum) with a -// chosen hrp amount, timestamp, 'h' (23) and 'x' (6) tags, and a zero -// signature — good enough to exercise our validation-only decoder -function craftInvoice({ amountHrp = '10u', timestamp = Math.floor(Date.now() / 1000), hHex = null, expiry = null }) { +// chosen hrp amount, timestamp, 'p' payment_hash (required), 'h' (23) and +// 'x' (6) tags, and a zero signature. NOTE ON SIGNATURES: the decoder +// intentionally does not verify the ECDSA signature — for invoices without +// an 'n' payee tag (the common case), public-key recovery "succeeds" for any +// input by construction, so it adds no integrity beyond the bech32 checksum; +// the payee *is* the recovered key. Real integrity before money moves is +// enforced by (a) our checksum/shape/amount/binding checks here and (b) the +// paying wallet's full validation — both the NWC payment path and any wallet +// scanning the recovery QR validate signatures themselves. +function craftInvoice({ amountHrp = '10u', timestamp = Math.floor(Date.now() / 1000), hHex = null, expiry = null, hashHex = 'ab'.repeat(32) }) { const words = []; const pushInt = (n, wordCount) => { const bin = n.toString(2).padStart(wordCount * 5, '0'); for (let i = 0; i < wordCount; i++) words.push(parseInt(bin.slice(i * 5, i * 5 + 5), 2)); }; - pushInt(timestamp, 7); // timestamp: 7 words - if (hHex) { - const value = Buffer.from(hHex, 'hex'); - // bytes → 5-bit words, right-padded + const pushBytes = (value) => { let acc = 0, bits = 0; const vwords = []; for (const b of value) { acc = (acc << 8) | b; bits += 8; while (bits >= 5) { bits -= 5; vwords.push((acc >> bits) & 31); } } if (bits) vwords.push((acc << (5 - bits)) & 31); + return vwords; + }; + pushInt(timestamp, 7); // timestamp: 7 words + { // 'p' payment_hash (tag 1): required for a receivable invoice + const value = Buffer.from(hashHex, 'hex'); + const vwords = pushBytes(value); + words.push(1); pushInt(vwords.length, 2); words.push(...vwords); + } + if (hHex) { + const value = Buffer.from(hHex, 'hex'); + const vwords = pushBytes(value); words.push(23); // 'h' pushInt(vwords.length, 2); // length in 5-bit words (2 words BE) words.push(...vwords); @@ -145,6 +160,8 @@ test('non-mainnet hrp is rejected', async () => { const words = []; const pushInt = (n, c) => { const b = n.toString(2).padStart(c * 5, '0'); for (let i = 0; i < c; i++) words.push(parseInt(b.slice(i * 5, i * 5 + 5), 2)); }; pushInt(Math.floor(Date.now() / 1000), 7); + // required payment_hash tag (1 word type + 2 len + 52 words) + words.push(1); pushInt(52, 2); for (let i = 0; i < 52; i++) words.push(0); for (let i = 0; i < 104; i++) words.push(0); const hrp = 'lntb10u'; const pm = bech32Polymod(hrpExpand(hrp).concat(words, [0, 0, 0, 0, 0, 0])) ^ 1; @@ -155,6 +172,55 @@ test('non-mainnet hrp is rejected', async () => { assert.match(r.error, /mainnet/); }); +test('invoice missing the payment hash tag is rejected', async () => { + // hand-build an otherwise-valid invoice without tag 'p' + const words = []; + const pushInt = (n, c) => { const b = n.toString(2).padStart(c * 5, '0'); for (let i = 0; i < c; i++) words.push(parseInt(b.slice(i * 5, i * 5 + 5), 2)); }; + pushInt(Math.floor(Date.now() / 1000), 7); + for (let i = 0; i < 104; i++) words.push(0); + const hrp = 'lnbc10u'; + const pm = bech32Polymod(hrpExpand(hrp).concat(words, [0, 0, 0, 0, 0, 0])) ^ 1; + const cs = [...Array(6)].map((_, i) => (pm >> 5 * (5 - i)) & 31); + const inv = hrp + '1' + [...words, ...cs].map(d => CHARSET[d]).join(''); + const r = await core.decodeBolt11(inv); + assert.strictEqual(r.ok, false); + assert.match(r.error, /payment hash/); +}); + +test('mixed-case invoice is rejected', async () => { + const inv = craftInvoice({ amountHrp: '10u', hHex: sha256Hex('[]') }); + // flip one data character to upper — mixed case is invalid bech32 + const mixed = inv.slice(0, 12) + inv[12].toUpperCase() + inv.slice(13); + const r = await core.decodeBolt11(mixed); + assert.strictEqual(r.ok, false); + assert.match(r.error, /mixed case/); +}); + +// ---------- zap amount policy ---------- + +test('recipient minimum above the intended zap aborts (never auto-raises)', () => { + // hostile/hostile-ish endpoint: min 100,000 sats when we intend 1,000 + const r = core.resolveZapAmount({ intendedMsat: 1000 * 1000, minSendable: 100000 * 1000, maxSendable: 1000000 * 1000 }); + assert.strictEqual(r.error !== undefined, true); + assert.match(r.error, /at least 100000 sats/); + assert.strictEqual(r.msat, undefined); // nothing to pay +}); + +test('amount within the advertised range passes unchanged', () => { + const r = core.resolveZapAmount({ intendedMsat: 1000 * 1000, minSendable: 1000, maxSendable: 10000000 * 1000 }); + assert.deepStrictEqual(r, { msat: 1000 * 1000 }); +}); + +test('recipient maximum below the intended zap clamps down', () => { + const r = core.resolveZapAmount({ intendedMsat: 1000 * 1000, minSendable: 1000, maxSendable: 500 * 1000 }); + assert.deepStrictEqual(r, { msat: 500 * 1000, adjustedDown: true }); +}); + +test('inverted recipient range is rejected', () => { + const r = core.resolveZapAmount({ intendedMsat: 1000 * 1000, minSendable: 5000 * 1000, maxSendable: 1000 * 1000 }); + assert.match(r.error, /invalid amount range/); +}); + // ---------- error classification ---------- test('transient payment errors are classified for recovery', () => { diff --git a/zap-core.js b/zap-core.js index ad611bd..e900713 100644 --- a/zap-core.js +++ b/zap-core.js @@ -80,7 +80,10 @@ const bad = (error) => ({ ok: false, error }); try { if (typeof invoice !== 'string') return bad('not a string'); - const inv = invoice.replace(/^lightning:/i, '').trim().toLowerCase(); + const stripped = invoice.replace(/^lightning:/i, '').trim(); + // bech32: all-lower or all-upper only — mixed case is invalid + if (/[a-z]/.test(stripped) && /[A-Z]/.test(stripped)) return bad('mixed case'); + const inv = stripped.toLowerCase(); const sep = inv.lastIndexOf('1'); if (sep < 4 || sep === inv.length - 1) return bad('missing separator'); const hrp = inv.slice(0, sep); @@ -113,17 +116,22 @@ // (words includes the trailing 6 bech32 checksum words — excluded here) const sigAt = words.length - 6 - 104; const timestamp = parseInt(words.slice(0, 7).map(w => w.toString(2).padStart(5, '0')).join('') || '0', 2); - let expiry = null, descriptionHash = null; + let expiry = null, descriptionHash = null, hasPaymentHash = false; let pos = 7; while (pos + 3 <= sigAt) { const type = words[pos]; const len = words[pos + 1] * 32 + words[pos + 2]; if (pos + 3 + len > sigAt) return bad('truncated tag'); const value = words.slice(pos + 3, pos + 3 + len); + if (type === 1 && len === 52) hasPaymentHash = true; // 'p' payment_hash (32 bytes) if (type === 23) descriptionHash = wordsToTrimmedHex(value); // 'h' purpose_commit_hash else if (type === 6) expiry = parseInt(value.map(w => w.toString(2).padStart(5, '0')).join('') || '0', 2); // 'x' expire_time pos += 3 + len; } + // a receivable BOLT11 invoice must commit to a payment hash — + // wallets reject invoice without one, and we must not present + // such QRs in recovery either + if (!hasPaymentHash) return bad('missing payment hash'); // expiry (default 3600) with a small clock-skew allowance const expiryAt = timestamp + (expiry ?? 3600) + 60; @@ -188,6 +196,23 @@ }; } + // ---------- zap amount policy ---------- + // The recipient controls minSendable/maxSendable — never let it raise the + // amount silently: a hostile "min" above the intended zap must abort, not + // auto-pay more (the amount-vs-invoice check can't catch this on its own, + // because the invoice would match the inflated request). + function resolveZapAmount({ intendedMsat, minSendable, maxSendable }) { + const min = Number(minSendable) || 0; + const max = Number.isFinite(maxSendable) ? Number(maxSendable) : Infinity; + if (min && max && min > max) return { error: 'recipient advertises an invalid amount range' }; + if (min && intendedMsat < min) { + return { error: `recipient requires at least ${Math.ceil(min / 1000)} sats — more than this zap. Open the ⚡ dialog to pay them directly.` }; + } + // clamping *down* is safe (never pays more than intended) + if (max && intendedMsat > max) return { msat: Math.floor(max), adjustedDown: true }; + return { msat: Math.trunc(intendedMsat) }; + } + // ---------- provider selection ---------- // "Most recently connected provider wins". Covers: extension webln fails → // user reconnects via Bitcoin Connect → fresh bcProvider must be preferred. @@ -217,7 +242,7 @@ return { lnurlpUrl, lnurlEncode, escapeHtml, sha256Hex, decodeBolt11, createProviderSelector, - isTransientPaymentError, createZapGuard, + isTransientPaymentError, createZapGuard, resolveZapAmount, __internals: { CHARSET, bech32Polymod, hrpExpand, convertBits, wordsToTrimmedHex } }; }); diff --git a/zap.js b/zap.js index cd87d3d..e5db753 100644 --- a/zap.js +++ b/zap.js @@ -7,7 +7,7 @@ (() => { // shared, unit-tested core (zap-core.js must load before this script) const { lnurlpUrl, lnurlEncode, escapeHtml, sha256Hex, decodeBolt11, createProviderSelector, - isTransientPaymentError, createZapGuard } = zapCore; + isTransientPaymentError, createZapGuard, resolveZapAmount } = zapCore; const ZAP_SATS = 1000; // default zap amount for chips const ZAP_MEMO = 'nostr.net zap'; // so recipients know where it came from @@ -177,22 +177,27 @@ msg = `⚡ Sent ${escapeHtml(name)} ${sats} sats (no zap receipt)`; } toast(msg, 'success', 6000); - // one-time opt-in for attributed zaps — only meaningful when receipts bind - if (anon && receiptBound && window.nostr?.signEvent && localStorage.getItem(SIGN_AS_ME_KEY) !== '1') { - const t = document.querySelector('.zap-toast'); - if (t) { - const a = document.createElement('a'); - a.href = '#'; - a.className = 'zap-as-me'; - a.textContent = 'zap as me instead'; - a.addEventListener('click', (ev) => { - ev.preventDefault(); - localStorage.setItem(SIGN_AS_ME_KEY, '1'); - toast('Zaps will now be signed with your npub ⚡', 'success', 3000); - }); - t.appendChild(document.createTextNode(' · ')); - t.appendChild(a); + // one-time opt-in for attributed zaps — preference handling must never + // be able to report a successful payment as failed + try { + if (anon && receiptBound && window.nostr?.signEvent && localStorage.getItem(SIGN_AS_ME_KEY) !== '1') { + const t = document.querySelector('.zap-toast'); + if (t) { + const a = document.createElement('a'); + a.href = '#'; + a.className = 'zap-as-me'; + a.textContent = 'zap as me instead'; + a.addEventListener('click', (ev) => { + ev.preventDefault(); + try { localStorage.setItem(SIGN_AS_ME_KEY, '1'); } catch (e) { /* non-fatal */ } + toast('Zaps will now be signed with your npub ⚡', 'success', 3000); + }); + t.appendChild(document.createTextNode(' · ')); + t.appendChild(a); + } } + } catch (prefErr) { + console.warn('post-payment preference handling failed (payment already sent):', prefErr); } } @@ -242,9 +247,6 @@ if (chip) { chip.dataset.zapping = '1'; chip.dataset.state = 'zapping'; } try { await payInvoice(invoice, provider); - el.remove(); - guard.recoveryCleared(); - finishZap({ chip, msat, event, anon, name, receiptBound }); } catch (retryErr) { console.error('retry failed:', retryErr); btn.disabled = false; @@ -257,6 +259,16 @@ closeRecovery(); toast(`Couldn\u2019t zap ${escapeHtml(name)}: ${escapeHtml(String(retryErr?.message || retryErr).slice(0, 140))}`, 'error', 7000); } + return; + } + // paid — same rule as the direct path: never report failure now + el.remove(); + guard.recoveryCleared(); + try { + finishZap({ chip, msat, event, anon, name, receiptBound }); + } catch (uiErr) { + console.error('post-payment UI failed (payment WAS sent):', uiErr); + toast(`⚡ Payment sent — ${(msat / 1000)} sats to ${escapeHtml(name)}.`, 'success', 8000); } }); @@ -283,7 +295,7 @@

${Math.round(msat / 1000)} sats → ${escapeHtml(name)}

${receiptBound - ? 'Scan with any wallet — the zap request is baked into this invoice; paying it publishes the zap receipt. Valid for a few minutes.' + ? 'Scan with any wallet — this invoice is bound to the zap request; payment should publish the receipt via the recipient\u2019s server. Valid for a few minutes.' : 'Scan with any wallet to pay. This provider doesn’t bind zap receipts to invoices, so the zap may not appear in feeds.'}

@@ -325,9 +337,16 @@ ]); let msat = ZAP_SATS * 1000; - const min = params.minSendable || 1000, max = params.maxSendable || Infinity; - const clamped = Math.min(Math.max(msat, min), max); - if (clamped !== msat) { msat = clamped; toast(`Amount adjusted to ${msat / 1000} sats (wallet limits)`, 'info'); } + // recipient-controlled range: never raise the amount silently — + // a hostile minimum above the intended zap aborts (no auto-pay) + const amt = resolveZapAmount({ + intendedMsat: msat, + minSendable: params.minSendable, + maxSendable: params.maxSendable + }); + if (amt.error) throw new Error(amt.error); + msat = amt.msat; + if (amt.adjustedDown) toast(`Amount adjusted to ${msat / 1000} sats (recipient's maximum)`, 'info', 4000); const { event, anon } = await buildZapRequest(params, msat, lnAddress); const inv = await fetchInvoice(params, msat, event); @@ -348,7 +367,15 @@ } return; } - finishZap({ chip, msat, event, anon, name, receiptBound: inv.receiptBound }); + // payment succeeded — from here on, NO failure path may claim the + // zap failed (that invites double-payment); UI hiccups degrade to + // a plain success notice + try { + finishZap({ chip, msat, event, anon, name, receiptBound: inv.receiptBound }); + } catch (uiErr) { + console.error('post-payment UI failed (payment WAS sent):', uiErr); + toast(`⚡ Payment sent — ${(msat / 1000)} sats to ${escapeHtml(name)}.`, 'success', 8000); + } } catch (err) { console.error('zap failed:', err); if (chip) { chip.dataset.state = 'error'; delete chip.dataset.zapping; setTimeout(() => { if (chip.dataset.state === 'error') delete chip.dataset.state; }, 2500); }