diff --git a/frontend/src/composables/useWallet.ts b/frontend/src/composables/useWallet.ts index 689bd2d..094e634 100644 --- a/frontend/src/composables/useWallet.ts +++ b/frontend/src/composables/useWallet.ts @@ -64,7 +64,6 @@ export function useWallet() { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ - pubkey: pubkey.value, method: 'nwc', connectionData: connectionString, }), @@ -93,7 +92,6 @@ export function useWallet() { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ - pubkey: pubkey.value, method: 'lnaddress', connectionData: address, }), @@ -114,8 +112,6 @@ export function useWallet() { await authFetch('/api/payments/disconnect-wallet', { method: 'DELETE', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ pubkey: pubkey.value }), }) walletMethod.value = null @@ -129,7 +125,7 @@ export function useWallet() { async function checkWalletStatus(): Promise { if (!pubkey.value) return - const res = await authFetch(`/api/payments/wallet-status?pubkey=${pubkey.value}`) + const res = await authFetch('/api/payments/wallet-status') if (res.ok) { const data = await res.json() isWalletConnected.value = data.connected @@ -148,7 +144,7 @@ export function useWallet() { const invoiceRes = await authFetch('/api/payments/create-invoice', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ botId, pubkey: pubkey.value }), + body: JSON.stringify({ botId }), }) if (!invoiceRes.ok) { @@ -179,7 +175,7 @@ export function useWallet() { await authFetch(`/api/payments/confirm/${paymentId}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ preimage, pubkey: pubkey.value }), + body: JSON.stringify({ preimage }), }) paymentStatus.value = 'confirmed' return paymentId diff --git a/frontend/src/pages/JoinBoutPage.vue b/frontend/src/pages/JoinBoutPage.vue index f4fc102..1e28b67 100644 --- a/frontend/src/pages/JoinBoutPage.vue +++ b/frontend/src/pages/JoinBoutPage.vue @@ -717,10 +717,12 @@ async function fightRanked() { // don't create a duplicate Lightning invoice via payEntryFee(). const paymentId = cashuPaymentId.value ?? await payEntryFee(bot.value.id) cashuPaymentId.value = null + // Ownership is verified server-side from the Bearer JWT that authFetch + // attaches automatically — no client-supplied pubkey needed (or trusted). const res = await authFetch(`/api/queue/join-ranked/${bot.value.id}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ paymentId, pubkey: pubkey.value }), + body: JSON.stringify({ paymentId }), }) if (res.ok) { const data = await res.json() diff --git a/server/src/lib/validators.ts b/server/src/lib/validators.ts index 2928ae1..bcb4e5f 100644 --- a/server/src/lib/validators.ts +++ b/server/src/lib/validators.ts @@ -88,14 +88,15 @@ export const withdrawSchema = z.object({ // --- Payment schemas --- export const connectWalletSchema = z.object({ - pubkey: pubkeySchema, method: z.enum(['nwc', 'lnaddress', 'cashu_mint']), connectionData: z.string().min(1), }) +// pubkey is intentionally NOT part of this schema — see connect-wallet / +// create-invoice / claim / disconnect-wallet in payments.ts, which all +// derive ownership from the verified JWT, never a client-supplied field. export const createInvoiceSchema = z.object({ botId: idSchema, - pubkey: pubkeySchema.optional(), }) export const submitCashuSchema = z.object({ @@ -109,9 +110,7 @@ export const zapSchema = z.object({ amountSats: satsSchema, }) -export const disconnectWalletSchema = z.object({ - pubkey: pubkeySchema, -}) +export const disconnectWalletSchema = z.object({}) // --- Tournament schemas --- @@ -134,9 +133,11 @@ export const startTournamentSchema = z.object({ // --- Queue schemas --- +// pubkey is intentionally NOT part of this schema — ownership is verified +// server-side via verifyBotOwner (JWT-derived pubkey or bot-secret), never +// from a client-supplied field. See queue.ts. export const joinRankedSchema = z.object({ paymentId: idSchema, - pubkey: pubkeySchema.optional(), }) // --- Docs schemas --- diff --git a/server/src/middleware/bot-auth.ts b/server/src/middleware/bot-auth.ts index 1ee1935..85e3493 100644 --- a/server/src/middleware/bot-auth.ts +++ b/server/src/middleware/bot-auth.ts @@ -6,6 +6,7 @@ import { createHash, timingSafeEqual } from 'crypto' import { db, schema } from '../db/index.js' import { eq } from 'drizzle-orm' import type { Context } from 'hono' +import { extractPubkeyFromAuth } from './jwt.js' export interface BotAuthContext { botId: string @@ -68,3 +69,40 @@ export async function authenticateBot(c: Context): Promise:, which never have a publicKey — see + * BOTFIGHTS.md). This is the ONLY correct way to check the nostr side: it + * derives pubkey from a verified JWT (extractPubkeyFromAuth), never from a + * client-supplied `pubkey` field. A bare `body.pubkey === bot.publicKey` + * comparison is not an ownership check at all — pubkeys are public by + * design in nostr (shown on every bot's own profile page), so anyone who's + * viewed a bot's page could pass that same auth-check with zero secret + * material. (This exact bug, at POST /api/auth/update, was found and fixed + * in 09-06 — see auth.ts. Same class, same fix, applied everywhere ownership + * is checked by pubkey.) + */ +export async function verifyBotOwner(c: Context, botId: string): Promise { + const auth = c.req.header('Authorization') + if (auth?.startsWith('Bearer ')) { + const pubkey = extractPubkeyFromAuth(auth) + if (!pubkey) { + return c.json({ error: 'Invalid or expired session.' }, 401) + } + const rows = await db.select({ publicKey: schema.bots.publicKey }) + .from(schema.bots).where(eq(schema.bots.id, botId)).limit(1) + if (rows.length === 0 || rows[0].publicKey !== pubkey) { + return c.json({ error: 'Unauthorized' }, 403) + } + return true + } + + const botOrRes = await authenticateBot(c) + if (botOrRes instanceof Response) return botOrRes + if (botOrRes.botId !== botId) { + return c.json({ error: 'Unauthorized' }, 403) + } + return true +} diff --git a/server/src/routes/payments.test.ts b/server/src/routes/payments.test.ts index 3d985f6..acf9fc1 100644 --- a/server/src/routes/payments.test.ts +++ b/server/src/routes/payments.test.ts @@ -56,6 +56,16 @@ vi.mock('../middleware/rate-limit.js', () => ({ const { paymentsRouter } = await import('./payments.js') const { db, schema } = await import('../db/index.js') const { createEntryInvoice, checkPaymentStatus } = await import('../engine/payments.js') +// jwt.js is intentionally NOT mocked — connect-wallet, disconnect-wallet, +// wallet-status, winnings, and claim all derive identity from a real, +// verified JWT (see 09-06 IDOR fix), so tests that exercise the +// authenticated path need a real token, not a stubbed one. +const { createJwt } = await import('../middleware/jwt.js') + +const TEST_PUBKEY = 'a'.repeat(64) +function authHeader(pubkey = TEST_PUBKEY) { + return { Authorization: `Bearer ${createJwt(pubkey)}` } +} function makeApp() { const app = new Hono() @@ -68,12 +78,22 @@ describe('payments routes', () => { vi.clearAllMocks() }) - it('connect-wallet returns 400 when missing fields', async () => { + it('connect-wallet returns 401 with no Authorization header', async () => { const app = makeApp() const res = await app.request('/api/payments/connect-wallet', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ pubkey: 'abc' }), + body: JSON.stringify({ method: 'nwc', connectionData: 'x' }), + }) + expect(res.status).toBe(401) + }) + + it('connect-wallet returns 400 when missing fields', async () => { + const app = makeApp() + const res = await app.request('/api/payments/connect-wallet', { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...authHeader() }, + body: JSON.stringify({}), }) expect(res.status).toBe(400) const json = await res.json() as { error: string } @@ -85,9 +105,8 @@ describe('payments routes', () => { // db.select will return empty array by default const res = await app.request('/api/payments/connect-wallet', { method: 'POST', - headers: { 'Content-Type': 'application/json' }, + headers: { 'Content-Type': 'application/json', ...authHeader() }, body: JSON.stringify({ - pubkey: 'a'.repeat(64), method: 'nwc', connectionData: 'nostr+walletconnect://test', }), @@ -160,14 +179,14 @@ describe('payments routes', () => { expect(json.error).toContain('Missing') }) - it('disconnect-wallet returns 400 when missing pubkey', async () => { + it('disconnect-wallet returns 401 with no Authorization header', async () => { const app = makeApp() const res = await app.request('/api/payments/disconnect-wallet', { method: 'DELETE', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({}), }) - expect(res.status).toBe(400) + expect(res.status).toBe(401) }) it('disconnect-wallet wipes connection data and sets hasWallet=false', async () => { @@ -184,8 +203,8 @@ describe('payments routes', () => { const app = makeApp() const res = await app.request('/api/payments/disconnect-wallet', { method: 'DELETE', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ pubkey: 'a'.repeat(64) }), + headers: { 'Content-Type': 'application/json', ...authHeader() }, + body: JSON.stringify({}), }) expect(res.status).toBe(200) const json = await res.json() as { success: boolean } @@ -314,9 +333,8 @@ describe('payment security — attack vectors', () => { const app = makeApp() const res = await app.request('/api/payments/connect-wallet', { method: 'POST', - headers: { 'Content-Type': 'application/json' }, + headers: { 'Content-Type': 'application/json', ...authHeader() }, body: JSON.stringify({ - pubkey: 'a'.repeat(64), method: 'paypal', connectionData: 'malicious://data', }), diff --git a/server/src/routes/payments.ts b/server/src/routes/payments.ts index 8269f5d..6652dd8 100644 --- a/server/src/routes/payments.ts +++ b/server/src/routes/payments.ts @@ -6,17 +6,32 @@ import { eq } from 'drizzle-orm' import { createEntryInvoice, checkPaymentStatus, redeemCashuToken } from '../engine/payments.js' import { encrypt, decrypt } from '../engine/crypto.js' import { rateLimit } from '../middleware/rate-limit.js' -import { connectWalletSchema, createInvoiceSchema, submitCashuSchema, disconnectWalletSchema, zapSchema, formatZodError, sanitizeError } from '../lib/validators.js' +import { connectWalletSchema, createInvoiceSchema, submitCashuSchema, zapSchema, formatZodError, sanitizeError } from '../lib/validators.js' +import { extractPubkeyFromAuth } from '../middleware/jwt.js' export const paymentsRouter = new Hono() // POST /connect-wallet +// +// SECURITY: pubkey MUST come from the verified JWT, never the request body. +// This handler previously trusted a client-supplied `pubkey` field with NO +// ownership check at all — any unauthenticated caller could attach an +// attacker-controlled NWC connection string or Lightning Address to ANY +// victim bot by pubkey (public by design in nostr), silently redirecting +// all of that bot's future fight-winnings payouts to the attacker's own +// wallet. Direct fund theft, not just profile hijacking. Found and fixed +// alongside the identical pattern at POST /api/auth/update (09-06). paymentsRouter.post('/connect-wallet', async (c) => { + const pubkey = extractPubkeyFromAuth(c.req.header('Authorization')) + if (!pubkey) { + return c.json({ error: 'Authentication required.' }, 401) + } + const parsed = connectWalletSchema.safeParse(await c.req.json().catch(() => ({}))) if (!parsed.success) { - return c.json({ error: formatZodError(parsed.error, {}, 'Missing pubkey, method, or connectionData') }, 400) + return c.json({ error: formatZodError(parsed.error, {}, 'Missing method or connectionData') }, 400) } - const { pubkey, method, connectionData } = parsed.data + const { method, connectionData } = parsed.data // Look up bot by publicKey const botRows = await db.select({ id: schema.bots.id }) @@ -59,9 +74,11 @@ paymentsRouter.post('/connect-wallet', async (c) => { return c.json({ success: true }) }) -// GET /wallet-status +// GET /wallet-status — read-only, but still derives identity from the JWT +// rather than a query-string pubkey, so this can't be used to enumerate +// whether an arbitrary victim pubkey has a wallet connected. paymentsRouter.get('/wallet-status', async (c) => { - const pubkey = c.req.query('pubkey') + const pubkey = extractPubkeyFromAuth(c.req.header('Authorization')) if (!pubkey) return c.json({ connected: false, method: null }) const botRows = await db.select({ id: schema.bots.id }) @@ -85,12 +102,14 @@ paymentsRouter.get('/wallet-status', async (c) => { paymentsRouter.post('/create-invoice', rateLimit(60_000, 10), async (c) => { const parsed = createInvoiceSchema.safeParse(await c.req.json().catch(() => ({}))) if (!parsed.success) return c.json({ error: formatZodError(parsed.error, { botId: 'Missing botId' }, 'Missing botId') }, 400) - const { botId, pubkey } = parsed.data + const { botId } = parsed.data - // In production, verify bot ownership + // In production, verify bot ownership via the verified JWT — never a + // client-supplied pubkey field (same fix class as connect-wallet above). if (process.env.NODE_ENV === 'production') { - if (!pubkey || typeof pubkey !== 'string' || pubkey.length !== 64) { - return c.json({ error: 'Missing pubkey' }, 400) + const pubkey = extractPubkeyFromAuth(c.req.header('Authorization')) + if (!pubkey) { + return c.json({ error: 'Authentication required.' }, 401) } const botRows = await db.select({ publicKey: schema.bots.publicKey }) .from(schema.bots).where(eq(schema.bots.id, botId)).limit(1) @@ -130,7 +149,7 @@ paymentsRouter.post('/confirm/:paymentId', rateLimit(60_000, 20), async (c) => { return c.json({ error: 'Invalid paymentId' }, 400) } - const { preimage, pubkey } = await c.req.json<{ preimage?: string; pubkey?: string }>().catch(() => ({ preimage: undefined, pubkey: undefined })) + const { preimage } = await c.req.json<{ preimage?: string }>().catch(() => ({ preimage: undefined })) const rows = await db.select().from(schema.payments) .where(eq(schema.payments.id, paymentId)).limit(1) @@ -143,15 +162,19 @@ paymentsRouter.post('/confirm/:paymentId', rateLimit(60_000, 20), async (c) => { // Must be an inbound entry payment if (payment.direction !== 'in') return c.json({ error: 'Cannot confirm outbound payments' }, 400) - // Verify caller owns this payment's bot - if (pubkey && typeof pubkey === 'string' && pubkey.length === 64) { + // Verify caller owns this payment's bot — pubkey comes from the verified + // JWT, never a client-supplied field (same fix class as connect-wallet + // above: a bare body.pubkey === bot.publicKey check is not an ownership + // proof, since pubkeys are public by design in nostr). + const pubkey = extractPubkeyFromAuth(c.req.header('Authorization')) + if (pubkey) { const botRows = await db.select({ publicKey: schema.bots.publicKey }) .from(schema.bots).where(eq(schema.bots.id, payment.botId)).limit(1) if (botRows.length === 0 || botRows[0].publicKey !== pubkey) { return c.json({ error: 'Unauthorized' }, 403) } } else if (process.env.NODE_ENV === 'production') { - return c.json({ error: 'Missing or invalid pubkey' }, 400) + return c.json({ error: 'Authentication required.' }, 401) } // In production, also verify payment via NWC lookup (belt and suspenders) @@ -204,18 +227,42 @@ paymentsRouter.post('/submit-cashu', async (c) => { }) // GET /winnings/:botId +// +// SECURITY (critical): this handler previously had NO auth check at all AND +// returned the raw, spendable Cashu bearer token in the list response. +// botId is public (appears in every fight/profile URL), so anyone could +// list ANY bot's unclaimed winnings and get the live token back — +// no ownership proof needed whatsoever. Whoever holds a Cashu token can +// redeem it, so this leaked real, spendable sats to any caller who beat the +// legitimate winner to the request. Fixed: require JWT-derived ownership of +// botId, and never include the token itself in the list — only reveal it +// via the explicit POST /claim/:paymentId below, which also clears it from +// storage (single-use reveal, correct claim semantics). paymentsRouter.get('/winnings/:botId', async (c) => { const botId = c.req.param('botId') + const pubkey = extractPubkeyFromAuth(c.req.header('Authorization')) + if (!pubkey) { + return c.json({ error: 'Authentication required.' }, 401) + } + const botRows = await db.select({ publicKey: schema.bots.publicKey }) + .from(schema.bots).where(eq(schema.bots.id, botId)).limit(1) + if (botRows.length === 0 || botRows[0].publicKey !== pubkey) { + return c.json({ error: 'Unauthorized' }, 403) + } + const unclaimed = await db.select({ paymentId: schema.payments.id, - cashuToken: schema.payments.cashuToken, + hasToken: schema.payments.cashuToken, amountSats: schema.payments.amountSats, }).from(schema.payments) .where(eq(schema.payments.botId, botId)) - // Filter in JS since drizzle doesn't easily combine multiple conditions - const filtered = unclaimed.filter(p => p.cashuToken) + // Filter in JS since drizzle doesn't easily combine multiple conditions. + // Never include the raw token here — see comment above. + const filtered = unclaimed + .filter(p => p.hasToken) + .map(p => ({ paymentId: p.paymentId, amountSats: p.amountSats })) return c.json({ unclaimed: filtered }) }) @@ -223,7 +270,6 @@ paymentsRouter.get('/winnings/:botId', async (c) => { // POST /claim/:paymentId paymentsRouter.post('/claim/:paymentId', async (c) => { const paymentId = c.req.param('paymentId') - const { pubkey } = await c.req.json<{ pubkey?: string }>().catch(() => ({ pubkey: undefined })) const rows = await db.select().from(schema.payments) .where(eq(schema.payments.id, paymentId)) @@ -233,7 +279,11 @@ paymentsRouter.post('/claim/:paymentId', async (c) => { const payment = rows[0] - // Verify caller owns this payment's bot + // Verify caller owns this payment's bot — pubkey comes from the verified + // JWT, never a client-supplied field. See GET /winnings above for the + // severity rationale (this route hands back a live, spendable bearer + // token — the single most sensitive check in this file). + const pubkey = extractPubkeyFromAuth(c.req.header('Authorization')) if (pubkey) { const botRows = await db.select({ publicKey: schema.bots.publicKey }) .from(schema.bots).where(eq(schema.bots.id, payment.botId)).limit(1) @@ -241,7 +291,7 @@ paymentsRouter.post('/claim/:paymentId', async (c) => { return c.json({ error: 'Unauthorized' }, 403) } } else if (process.env.NODE_ENV === 'production') { - return c.json({ error: 'Missing pubkey' }, 400) + return c.json({ error: 'Authentication required.' }, 401) } if (!payment.cashuToken) return c.json({ error: 'No Cashu token to claim' }, 400) @@ -255,9 +305,10 @@ paymentsRouter.post('/claim/:paymentId', async (c) => { // DELETE /disconnect-wallet paymentsRouter.delete('/disconnect-wallet', async (c) => { - const parsed = disconnectWalletSchema.safeParse(await c.req.json().catch(() => ({}))) - if (!parsed.success) return c.json({ error: formatZodError(parsed.error, {}, 'Missing pubkey') }, 400) - const { pubkey } = parsed.data + const pubkey = extractPubkeyFromAuth(c.req.header('Authorization')) + if (!pubkey) { + return c.json({ error: 'Authentication required.' }, 401) + } const botRows = await db.select({ id: schema.bots.id }) .from(schema.bots) diff --git a/server/src/routes/queue.ts b/server/src/routes/queue.ts index d3d5a19..5cffa51 100644 --- a/server/src/routes/queue.ts +++ b/server/src/routes/queue.ts @@ -5,7 +5,7 @@ import { joinQueue, leaveQueue, getQueueSize, getQueueSnapshot } from '../engine import { joinRankedQueue, getRankedQueueStatus } from '../engine/ranked-queue.js' import { rateLimit } from '../middleware/rate-limit.js' import { joinRankedSchema, sanitizeError } from '../lib/validators.js' -import { authenticateBot } from '../middleware/bot-auth.js' +import { verifyBotOwner } from '../middleware/bot-auth.js' export const queueRouter = new Hono() @@ -57,7 +57,14 @@ queueRouter.get('/ranked-status', (c) => { // Join ranked queue — requires confirmed payment + bot ownership. // Ownership can be proven either way, since ranked/staked fights are for // BOTH audiences (not just nostr-signed-in humans): -// 1. pubkey (nostr-authenticated bots, the web UI's own JWT session flow) +// 1. Authorization: Bearer (nostr-authenticated bots, the web UI's +// own JWT session flow) — verified via verifyBotOwner, which derives +// pubkey from the JWT itself, never from a client-supplied field. A +// bare `body.pubkey === bot.publicKey` comparison (the previous +// implementation here) is not an ownership check: pubkeys are public +// by design in nostr, shown on every bot's own profile page, so it let +// anyone who'd seen a bot's page join ranked queue as that bot. Found +// and fixed alongside the identical bug at POST /api/auth/update (09-06). // 2. Authorization: Bot : (anonymous poll-mode bots — the // primary registration path for AI agents per BOTFIGHTS.md, which // never have a publicKey at all: confirmed live, publicKey is null @@ -69,25 +76,12 @@ queueRouter.post('/join-ranked/:botId', async (c) => { if (!parsed.success) { return c.json({ error: parsed.error.issues[0]?.message || 'Missing paymentId' }, 400) } - const { paymentId, pubkey } = parsed.data + const { paymentId } = parsed.data // Verify bot ownership in production if (process.env.NODE_ENV === 'production') { - if (pubkey && typeof pubkey === 'string' && pubkey.length === 64) { - const botRows = await db.select({ publicKey: schema.bots.publicKey }) - .from(schema.bots).where(eq(schema.bots.id, botId)).limit(1) - if (botRows.length === 0 || botRows[0].publicKey !== pubkey) { - return c.json({ error: 'Unauthorized' }, 403) - } - } else { - // No pubkey supplied — fall back to bot-secret auth (Authorization - // header or ?bot_id=&secret= query params, same as /api/fights/poll). - const botOrRes = await authenticateBot(c) - if (botOrRes instanceof Response) return botOrRes - if (botOrRes.botId !== botId) { - return c.json({ error: 'Unauthorized' }, 403) - } - } + const ownerCheck = await verifyBotOwner(c, botId) + if (ownerCheck !== true) return ownerCheck } try {