Skip to content

Commit 084c0da

Browse files
grunchclaude
andauthored
feat: enforce invoice amount matches order amount on payouts (#831)
* feat: enforce invoice amount matches order amount on payouts Adds defense-in-depth so the bot never pays an invoice whose amount differs from the order amount, closing a path where a malicious LNURL/Lightning Address endpoint could return an inflated invoice and be paid more than was held from the seller. - ln/pay_request.ts: payRequest refuses to pay when the invoice encodes an amount that does not equal the expected amount, returning an AMOUNT_MISMATCH error. payToBuyer treats AMOUNT_MISMATCH as a structural failure: it alerts and does NOT schedule a retry. - lnurl/lnurl-pay.ts: resolvLightningAddress parses the returned invoice and rejects it (returns false) unless it encodes exactly the requested msat amount, instead of trusting the server's min/maxSendable. - jobs/pending_payments.ts: stop retrying a pending payment whose amount no longer matches the order amount (marks it as non-retryable). Also fixes early `return` -> `continue` so one skipped pending payment no longer aborts processing of the rest of the queue. - bot/scenes.ts: in both addInvoice wizards, bail out with the unavailable_lightning_address notice when the LNURL resolution returns no usable invoice, instead of dereferencing a missing pr. - bot/commands.ts: correct the unavailable-address guard to also handle a null/undefined resolution result (!laRes || !laRes.pr). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: remove redundant pending.save() in amount-mismatch branch The finally block already persists pending on every iteration, so the explicit save inside the amount-mismatch guard was a duplicate DB write. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f3e6aa2 commit 084c0da

5 files changed

Lines changed: 91 additions & 4 deletions

File tree

bot/commands.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ const addInvoice = async (
195195
buyer.lightning_address,
196196
order.amount * 1000,
197197
);
198-
if (!!laRes && !laRes.pr) {
198+
if (!laRes || !laRes.pr) {
199199
logger.error(
200200
`lightning address ${buyer.lightning_address} not available`,
201201
);

bot/scenes.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ const addInvoiceWizard = new Scenes.WizardScene(
7373
lnInvoice,
7474
order.amount * 1000,
7575
);
76+
if (!laRes || !laRes.pr) {
77+
await ctx.reply(
78+
ctx.i18n.t('unavailable_lightning_address', { la: lnInvoice }),
79+
);
80+
return;
81+
}
7682
lnInvoice = laRes.pr;
7783
res.invoice = parsePaymentRequest({ request: lnInvoice });
7884
} else {
@@ -146,6 +152,12 @@ const addInvoicePHIWizard = new Scenes.WizardScene(
146152
lnInvoice,
147153
order.amount * 1000,
148154
);
155+
if (!laRes || !laRes.pr) {
156+
await ctx.reply(
157+
ctx.i18n.t('unavailable_lightning_address', { la: lnInvoice }),
158+
);
159+
return;
160+
}
149161
lnInvoice = laRes.pr;
150162
res.invoice = parsePaymentRequest({ request: lnInvoice });
151163
} else {

jobs/pending_payments.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export const attemptPendingPayments = async (
3535
pending.paid = true;
3636
await pending.save();
3737
logger.info(`Order id: ${order._id} was already paid`);
38-
return;
38+
continue;
3939
}
4040
// We check if the old payment is on flight
4141
const isPendingOldPayment: boolean = await isPendingPayment(
@@ -48,7 +48,20 @@ export const attemptPendingPayments = async (
4848
);
4949

5050
// If one of the payments is on flight we don't do anything
51-
if (isPending || isPendingOldPayment) return;
51+
if (isPending || isPendingOldPayment) continue;
52+
53+
// SECURITY (defense in depth): the amount to pay must equal the order
54+
// amount. payRequest also enforces this, but we stop retries early here
55+
// so a structurally-wrong pending payment is never re-attempted.
56+
if (pending.amount !== order.amount) {
57+
pending.last_error = 'AMOUNT_MISMATCH';
58+
pending.is_invoice_expired = true; // prevents further retries
59+
logger.error(
60+
`attemptPendingPayments: amount mismatch for order ${order._id} ` +
61+
`(pending ${pending.amount} != order ${order.amount}); stopping retries`,
62+
);
63+
continue;
64+
}
5265

5366
const payment = await payRequest({
5467
amount: pending.amount,
@@ -61,12 +74,13 @@ export const attemptPendingPayments = async (
6174
if (!!payment && payment.is_expired) {
6275
pending.is_invoice_expired = true;
6376
order.paid_hold_buyer_invoice_updated = false;
64-
return await messages.expiredInvoiceOnPendingMessage(
77+
await messages.expiredInvoiceOnPendingMessage(
6578
bot,
6679
buyerUser,
6780
order,
6881
i18nCtx,
6982
);
83+
continue;
7084
}
7185

7286
if (!!payment && !!payment.confirmed_at) {
@@ -112,6 +126,12 @@ export const attemptPendingPayments = async (
112126
logger.warning(
113127
`Routing failed for order ${order._id}, attempt ${pending.attempts}`,
114128
);
129+
} else if (payment.error === 'AMOUNT_MISMATCH') {
130+
// Structural error: never retry an invoice with the wrong amount.
131+
pending.is_invoice_expired = true;
132+
logger.error(
133+
`AMOUNT_MISMATCH for order ${order._id}; stopping retries`,
134+
);
115135
} else {
116136
logger.error(
117137
`Payment failed for order ${order._id}, attempt ${pending.attempts}, error: ${payment.error}`,

ln/pay_request.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,21 @@ const payRequest = async ({
4343
// If the invoice is expired we return is_expired = true
4444
if (invoice.is_expired) return invoice;
4545

46+
// SECURITY: never pay an amount different from the order amount.
47+
// If the invoice encodes an amount, it MUST match the expected amount
48+
// exactly. This prevents an attacker from supplying an inflated invoice
49+
// (e.g. via a malicious LNURL/Lightning Address endpoint) and draining
50+
// the node by being paid more than was held from the seller.
51+
if (invoice.tokens && invoice.tokens !== amount) {
52+
logger.error(
53+
`payRequest: invoice amount (${invoice.tokens}) does not match the expected amount (${amount}); refusing to pay`,
54+
);
55+
return {
56+
error: 'AMOUNT_MISMATCH',
57+
message: 'invoice amount does not match order amount',
58+
};
59+
}
60+
4661
const maxRoutingFee = process.env.MAX_ROUTING_FEE;
4762
if (maxRoutingFee === undefined)
4863
throw new Error('Environment variable MAX_ROUTING_FEE is not defined');
@@ -147,6 +162,21 @@ const payToBuyer = async (bot: HasTelegram, order: IOrder) => {
147162
);
148163
await messages.rateUserMessage(bot, buyerUser, order, i18nCtx);
149164
} else {
165+
// SECURITY: a structural amount mismatch must never be retried — the
166+
// invoice itself is wrong. Alert and stop instead of scheduling a retry,
167+
// otherwise the pending payments job would keep attempting to pay it.
168+
if (
169+
payment &&
170+
typeof payment === 'object' &&
171+
'error' in payment &&
172+
payment.error === 'AMOUNT_MISMATCH'
173+
) {
174+
logger.error(
175+
`payToBuyer: AMOUNT_MISMATCH for order ${order._id}; refusing to pay and not scheduling a retry`,
176+
);
177+
await messages.invoicePaymentFailedMessage(bot, buyerUser, i18nCtx);
178+
return;
179+
}
150180
// Handle different types of payment failures
151181
if (payment && typeof payment === 'object' && 'error' in payment) {
152182
const errorType = payment.error as string;

lnurl/lnurl-pay.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import axios from 'axios';
22
import { logger } from '../logger';
3+
// @ts-ignore
4+
const { parsePaymentRequest } = require('invoices');
35

46
// {
57
// pr: String, // bech32-serialized lightning invoice
@@ -28,6 +30,29 @@ const resolvLightningAddress = async (address: string, amountMsat: number) => {
2830
await axios.get(`${lnAddressRes.callback}${'?'}amount=${amountMsat}`)
2931
).data;
3032

33+
// SECURITY: do not trust the amount the server claims via min/maxSendable.
34+
// Verify the returned invoice actually encodes the exact requested amount,
35+
// otherwise a malicious LNURL endpoint could return an inflated invoice.
36+
if (res && res.pr) {
37+
try {
38+
const parsed = parsePaymentRequest({ request: res.pr });
39+
const prMsat = parsed.mtokens
40+
? Number(parsed.mtokens)
41+
: (parsed.tokens || 0) * 1000;
42+
if (prMsat !== amountMsat) {
43+
logger.error(
44+
`resolvLightningAddress: returned invoice amount ${prMsat} msat does not match requested ${amountMsat} msat`,
45+
);
46+
return false;
47+
}
48+
} catch (error) {
49+
logger.error(
50+
`resolvLightningAddress: could not parse returned invoice: ${error}`,
51+
);
52+
return false;
53+
}
54+
}
55+
3156
return res;
3257
};
3358

0 commit comments

Comments
 (0)