From f6549ef4b45d05e4254d0a04c5393b91890b6237 Mon Sep 17 00:00:00 2001 From: Georgy Butaev <41178744+g-but@users.noreply.github.com> Date: Fri, 28 Aug 2026 20:38:58 +0200 Subject: [PATCH] fix(payments): a seller's own wallet could be used to get them banned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/v1/pay/... and POST /api/v1/payments/public each mint a REAL Lightning invoice through the RECIPIENT's own LNURL/NWC relay, on every request. Both were limited per IP and nothing else, so an attacker rotating addresses could drive unbounded invoice creation against one seller — getting them rate-limited or banned by their own wallet provider, and littering their queue with orphan intents. The victim is the person who did nothing. Per-IP answers "is one caller hammering us". It cannot answer the question that protects a seller: "is one RECIPIENT's wallet being hammered", however many addresses it arrives from. rateLimitTipRecipient was built for exactly this — 20 invoices per 5 minutes against any one recipient — and its own doc says to apply it IN ADDITION to the per-IP limit. It was wired to tips and lnurlp only. rateLimitPaymentRecipient addresses an entity rather than a username and shares that budget, because it is the same victim's wallet either way. Ordering matters and differs per route: the L402 challenge has the recipient in its path, so the check sits beside the per-IP one; the public-support route only learns the recipient after the body parses, so it sits after parsing and BEFORE initiatePublicSupport, which is the call that actually mints the invoice. The verify-with-proof branch is deliberately left alone — it creates no intent (finding 7 covers its outbound cost separately). Residual, stated rather than hidden: a profile reachable both by username and by entity id has two buckets, so splitting across both paths doubles the budget. Collapsing them needs a username->entity lookup on the rate-limit path, a database round trip before deciding whether to serve at all. Twice a bounded number is still bounded; unbounded was the bug. bitbaum/orangecat#563 finding 1. Unblocked by #816 — until the key stopped being caller-controlled, no limiter here could be tripped at all. Mutation-proven: removing the guard fails exactly the two new tests and nothing else; restoring it passes all 12. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018waGt1ieA9TjpscqrbrnGb --- __tests__/unit/api/v1-pay-l402.test.ts | 25 +++++++++++++++++ __tests__/unit/api/v1-payments.test.ts | 1 + .../v1/pay/[entity_type]/[entity_id]/route.ts | 14 +++++++++- src/app/api/v1/payments/public/route.ts | 17 ++++++++++- src/lib/rate-limit.ts | 28 +++++++++++++++++++ 5 files changed, 83 insertions(+), 2 deletions(-) diff --git a/__tests__/unit/api/v1-pay-l402.test.ts b/__tests__/unit/api/v1-pay-l402.test.ts index 94dbfe956..dd3a0d7a5 100644 --- a/__tests__/unit/api/v1-pay-l402.test.ts +++ b/__tests__/unit/api/v1-pay-l402.test.ts @@ -64,10 +64,12 @@ jest.mock('@/domain/payments/l402', () => ({ jest.mock('@/lib/supabase/public', () => ({ createPublicClient: () => ({ _kind: 'public' }) })); jest.mock('@/lib/rate-limit', () => ({ rateLimitWriteAsync: jest.fn().mockResolvedValue({ success: true }), + rateLimitPaymentRecipient: jest.fn().mockResolvedValue({ success: true }), retryAfterSeconds: () => 1, })); import { GET } from '@/app/api/v1/pay/[entity_type]/[entity_id]/route'; +import { rateLimitPaymentRecipient } from '@/lib/rate-limit'; const ENTITY_ID = '11111111-2222-3333-4444-555555555555'; const params = Promise.resolve({ entity_type: 'product', entity_id: ENTITY_ID }); @@ -225,3 +227,26 @@ describe('GET /api/v1/pay/{type}/{id}', () => { expect(body.error.message).toContain('has not connected a Bitcoin wallet'); }); }); + +/** + * Finding 1 of bitbaum/orangecat#563: every challenge mints a REAL invoice + * through the recipient's own LNURL/NWC relay. A per-IP limit does not protect + * the seller from an attacker who rotates addresses — the victim's wallet + * provider is what gets hammered, and it is the victim who gets banned. + */ +describe('per-recipient invoice-spam limit', () => { + beforeEach(() => { + (rateLimitPaymentRecipient as jest.Mock).mockResolvedValue({ success: true }); + }); + + it('keys the limit on the recipient, not only the caller', async () => { + await GET(makeRequest('https://x.test/api/v1/pay/product/' + ENTITY_ID), { params }); + expect(rateLimitPaymentRecipient).toHaveBeenCalledWith('product', ENTITY_ID); + }); + + it('refuses with 429 when that recipient is already being hammered', async () => { + (rateLimitPaymentRecipient as jest.Mock).mockResolvedValue({ success: false }); + const res = await GET(makeRequest('https://x.test/api/v1/pay/product/' + ENTITY_ID), { params }); + expect(res.status).toBe(429); + }); +}); diff --git a/__tests__/unit/api/v1-payments.test.ts b/__tests__/unit/api/v1-payments.test.ts index 63b908037..83c198c42 100644 --- a/__tests__/unit/api/v1-payments.test.ts +++ b/__tests__/unit/api/v1-payments.test.ts @@ -59,6 +59,7 @@ jest.mock('@/domain/payments', () => ({ jest.mock('@/lib/rate-limit', () => ({ rateLimitWriteAsync: jest.fn().mockResolvedValue({ success: true }), + rateLimitPaymentRecipient: jest.fn().mockResolvedValue({ success: true }), rateLimitIntegrationKeyWrite: jest.fn().mockResolvedValue({ success: true }), rateLimitIntegrationKeyRead: jest.fn().mockResolvedValue({ success: true }), retryAfterSeconds: jest.fn().mockReturnValue(30), diff --git a/src/app/api/v1/pay/[entity_type]/[entity_id]/route.ts b/src/app/api/v1/pay/[entity_type]/[entity_id]/route.ts index fca51cfaa..8253ff519 100644 --- a/src/app/api/v1/pay/[entity_type]/[entity_id]/route.ts +++ b/src/app/api/v1/pay/[entity_type]/[entity_id]/route.ts @@ -10,7 +10,7 @@ import { createL402Challenge, verifyL402Payment } from '@/domain/payments/l402'; import { parseL402Authorization } from '@/domain/payments/l402-codec'; import { publicSupportCreateSchema } from '@/lib/validation/finance'; import { createPublicClient } from '@/lib/supabase/public'; -import { rateLimitWriteAsync, retryAfterSeconds } from '@/lib/rate-limit'; +import { rateLimitPaymentRecipient, rateLimitWriteAsync, retryAfterSeconds } from '@/lib/rate-limit'; import { logger } from '@/utils/logger'; import { clientIpKey } from '@/lib/client-ip'; @@ -86,6 +86,18 @@ export async function GET( ); } + // Per-IP is not enough here. Every challenge mints a REAL invoice through the + // recipient's own LNURL/NWC relay, so an attacker rotating IPs can still get + // a seller rate-limited or banned by their wallet provider. This bounds the + // damage to one recipient however many addresses it arrives from. + const recipientLimit = await rateLimitPaymentRecipient(entity_type, entity_id); + if (!recipientLimit.success) { + return apiRateLimited( + 'This page is receiving too many payment requests right now. Try again shortly.', + retryAfterSeconds(recipientLimit) + ); + } + const url = new URL(request.url); const parsed = publicSupportCreateSchema.safeParse({ entity_type, diff --git a/src/app/api/v1/payments/public/route.ts b/src/app/api/v1/payments/public/route.ts index f687a52f7..5dcbca334 100644 --- a/src/app/api/v1/payments/public/route.ts +++ b/src/app/api/v1/payments/public/route.ts @@ -7,7 +7,7 @@ import { import { initiatePublicSupport } from '@/domain/payments'; import { publicSupportCreateSchema } from '@/lib/validation/finance'; import { createPublicClient } from '@/lib/supabase/public'; -import { rateLimitWriteAsync, retryAfterSeconds } from '@/lib/rate-limit'; +import { rateLimitPaymentRecipient, rateLimitWriteAsync, retryAfterSeconds } from '@/lib/rate-limit'; import { logger } from '@/utils/logger'; import { clientIpKey } from '@/lib/client-ip'; @@ -30,6 +30,21 @@ export async function POST(request: Request) { return apiBadRequest('Invalid support request', parsed.error.errors); } + // After parsing, because the recipient is in the body — and BEFORE + // initiatePublicSupport, which is the call that mints a real invoice + // through the recipient's wallet. Per-IP alone leaves a seller exposed to + // an attacker who rotates addresses. + const recipientLimit = await rateLimitPaymentRecipient( + parsed.data.entity_type, + parsed.data.entity_id + ); + if (!recipientLimit.success) { + return apiRateLimited( + 'This page is receiving too many payment requests right now. Please try again shortly.', + retryAfterSeconds(recipientLimit) + ); + } + const result = await initiatePublicSupport(createPublicClient(), parsed.data); return apiSuccess(result, { status: 201, cache: 'NONE' }); } catch (error) { diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index b105d33d9..872f7d9a2 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -247,6 +247,34 @@ export async function rateLimitTipRecipient(username: string): Promise { + return rateLimitTipRecipient(`${entityType}:${entityId}`); +} + /** * Rate limit public Ask-Cat / feedback submissions per IP. * 8 per 5 minutes — each one is a platform LLM call from a possibly anonymous