From c07dfe050b7c79917744d7eddbf934d319cb9c89 Mon Sep 17 00:00:00 2001 From: Adam Soltys Date: Fri, 7 Aug 2026 23:00:07 -0700 Subject: payments: never lose a settled deposit to an oversized memo (S9/S10) credit() ran fail("memo too long") on memos >5000 chars, but it runs AFTER settlement: an incoming lightning/bolt12 deposit has already advanced pay_index, and an internal send has already debited the sender. The throw was only logged, replay() hit the same throw, and the sats sat in the house wallet uncredited. The memo is attacker-controllable and was uncapped via the lnurl comment param and the bolt12 payer_note. Truncate the memo in credit() instead of throwing (a clipped note is harmless), and cap the lnurl comment to the advertised 512 at ingress. Co-Authored-By: Claude Opus 4.8 --- lib/payments.ts | 9 ++++++++- routes/lnurl.ts | 5 ++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/payments.ts b/lib/payments.ts index 2fb891a2..960ff705 100644 --- a/lib/payments.ts +++ b/lib/payments.ts @@ -354,7 +354,14 @@ export const credit = async ({ tip = tip !== undefined ? Number.parseInt(tip) || 0 : Number.parseInt(inv.tip) || 0; if (!memo) ({ memo } = inv); - if (memo && memo.length > 5000) fail("memo too long"); + // Truncate rather than throw: credit() runs AFTER the payment has settled (an + // incoming lightning/bolt12 deposit already advanced pay_index; an internal + // send already debited the sender). Throwing here left the sats in the house + // wallet with no retry path — the payer saw a completed payment and the + // recipient was never credited. An oversized memo (attacker-controllable via + // the lnurl comment or a bolt12 payer_note) must never destroy a settled + // payment; a clipped note is harmless. + if (memo && memo.length > 5000) memo = memo.slice(0, 5000); if (amount < 0 || tip < 0) fail("Invalid amount"); // For a NON-internal receipt the tip is only a cosmetic split of the real // settled `amount` — there is no paired coinos debit carrying it — so it can diff --git a/routes/lnurl.ts b/routes/lnurl.ts index 1acc4d76..1b3f84c1 100644 --- a/routes/lnurl.ts +++ b/routes/lnurl.ts @@ -188,7 +188,10 @@ export default { }); if (comment) { - invoice.memo = comment; + // Enforce the commentAllowed:512 we advertise in the lnurlp response. + // This overwrite happens after generate()'s own memo validation, so an + // uncapped comment would otherwise reach credit() at settlement time. + invoice.memo = String(comment).slice(0, 512); await s(`invoice:${invoice.id}`, invoice); } -- cgit v1.2.3