From a8b4b9fdd091eec26a0d1eb5e1f140b64a32f3d1 Mon Sep 17 00:00:00 2001 From: SkateSpots Date: Fri, 2 Oct 2026 21:19:10 +0700 Subject: [PATCH] =?UTF-8?q?fix(security):=20tier=201=20=E2=80=94=20money?= =?UTF-8?q?=20&=20auth=20(C1=E2=80=93C5=20partial)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - C1 Marketplace payments now verify the invoice amount: usePayPaymentPointer calls assertInvoiceAmount(invoice, amountSats) before paying, so a seller can't attach a raw BOLT11 or return an inflated LNURL invoice and have an auto-approving wallet pay it. - C2 NIP-17 DMs now verifyEvent(seal) in all four read paths — the seal is the only authentication for a message; a crafted seal could impersonate the sender. - C3 push-service: bind endpoint ownership (reject subscribe by a different pubkey; only the owner may unsubscribe) and reject non-HTTPS/private/loopback endpoints (SSRF). - C4 Amber callback: a bare ?event= is no longer treated as a login; the callback must match a pending get_public_key op (both callback pages). Also stops logging the pending op payload. - C5a platform-fee record is now per-pubkey, so an account on a shared device can't inherit another's paid status. - C5b (self-asserted featured ranking) deferred: needs a batch-verification pass against provider zap receipts before it can gate ordering. --- push-service/server.ts | 52 ++++++++++++++++++-- src/components/auth/AmberCallbackHandler.tsx | 43 ++++------------ src/hooks/useDirectMessages.ts | 10 +++- src/hooks/usePayPaymentPointer.ts | 12 ++++- src/hooks/usePlatformFee.ts | 10 ++-- src/pages/AmberCallbackPage.tsx | 12 ++++- 6 files changed, 95 insertions(+), 44 deletions(-) diff --git a/push-service/server.ts b/push-service/server.ts index 9c13955..6a13f9a 100644 --- a/push-service/server.ts +++ b/push-service/server.ts @@ -142,8 +142,7 @@ function tagValue(ev: NostrEvent, name: string): string | undefined { } /** Verify a kind-27235 ownership proof for `expectedPubkey`, consuming its challenge. */ -function verifyProof(proof: unknown, expectedPubkey: string): { ok: boolean; error?: string } { - if (!proof || typeof proof !== 'object') return { ok: false, error: 'missing proof' }; +function verifyProof(proof: unknown, expectedPubkey: string): { ok: boolean; error?: string } { if (!proof || typeof proof !== 'object') return { ok: false, error: 'missing proof' }; const ev = proof as NostrEvent; if (ev.kind !== 27235) return { ok: false, error: 'proof must be kind 27235' }; if (ev.pubkey !== expectedPubkey) return { ok: false, error: 'proof pubkey mismatch' }; @@ -160,6 +159,38 @@ function verifyProof(proof: unknown, expectedPubkey: string): { ok: boolean; err return { ok: true }; } +/** + * Reject endpoints that could make the server POST to internal infrastructure + * (SSRF via a forged Web Push endpoint). Real push services are public HTTPS + * hosts; anything loopback/private/link-local is refused. + */ +function isSafePushEndpoint(endpoint: string): boolean { + let url: URL; + try { + url = new URL(endpoint); + } catch { + return false; + } + if (url.protocol !== 'https:') return false; + const host = url.hostname.toLowerCase(); + if (host === 'localhost' || host.endsWith('.localhost') || host.endsWith('.local') || host.endsWith('.internal')) { + return false; + } + const v4 = host.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/); + if (v4) { + const a = Number(v4[1]); + const b = Number(v4[2]); + if (a === 0 || a === 10 || a === 127) return false; + if (a === 172 && b >= 16 && b <= 31) return false; + if (a === 192 && b === 168) return false; + if (a === 169 && b === 254) return false; + if (a === 100 && b >= 64 && b <= 127) return false; + } + if (host === '[::1]' || host === '::1') return false; + if (host.startsWith('fe80:') || host.startsWith('fc') || host.startsWith('fd')) return false; + return true; +} + // ─── Web Push ──────────────────────────────────────────────────────────────── function subsFor(pubkey: string): StoredSubscription[] { @@ -413,9 +444,17 @@ Bun.serve({ if (!body?.proof) return json({ error: 'proof required' }, 400, headers); const check = verifyProof(body.proof, body.proof.pubkey); if (!check.ok) return json({ error: check.error }, 401, headers); + if (!isSafePushEndpoint(sub.endpoint)) { + return json({ error: 'invalid push endpoint' }, 400, headers); + } + const ownerPubkey = body.proof.pubkey.toLowerCase(); + const existing = state.subscriptions[sub.endpoint]; + if (existing && existing.pubkey !== ownerPubkey) { + return json({ error: 'endpoint already registered to another account' }, 409, headers); + } state.subscriptions[sub.endpoint] = { endpoint: sub.endpoint, - pubkey: body.proof.pubkey.toLowerCase(), + pubkey: ownerPubkey, p256dh: sub.keys.p256dh, auth: sub.keys.auth, categories: Array.isArray(body.categories) ? body.categories : undefined, @@ -431,6 +470,13 @@ Bun.serve({ if (!body?.endpoint || !body?.proof) return json({ error: 'endpoint and proof required' }, 400, headers); const check = verifyProof(body.proof, body.proof.pubkey); if (!check.ok) return json({ error: check.error }, 401, headers); + const owned = state.subscriptions[body.endpoint]; + // Only the owner may remove a subscription. A non-owner gets an idempotent + // OK without deleting anything, so this can't be used to disable another + // account's notifications or probe which endpoints exist. + if (owned && owned.pubkey !== body.proof.pubkey.toLowerCase()) { + return json({ ok: true }, 200, headers); + } delete state.subscriptions[body.endpoint]; save(); refreshMarketSubscription(); diff --git a/src/components/auth/AmberCallbackHandler.tsx b/src/components/auth/AmberCallbackHandler.tsx index 93b352a..89f057a 100644 --- a/src/components/auth/AmberCallbackHandler.tsx +++ b/src/components/auth/AmberCallbackHandler.tsx @@ -86,43 +86,18 @@ export function AmberCallbackHandler() { // Try to get pending operation from any storage const pendingOp = await getAmberPendingOperationAsync(); - console.log('[AmberCallback] Pending operation:', pendingOp); + // Log only the operation TYPE — never the payload (encrypt/decrypt ops can + // carry plaintext). + console.log('[AmberCallback] Pending operation type:', pendingOp?.type ?? 'none'); - // FALLBACK: If no pending operation found, but result looks like a valid pubkey, - // assume this is a login callback (PWA storage was cleared during redirect) const pubkeyRegex = /^[0-9a-fA-F]{64}$/; - + + // SECURITY: a callback is only valid when it matches an operation this app + // actually started. Never infer a login from a bare `?event=` URL — + // that would let any link switch the user into an arbitrary account. if (!pendingOp) { - console.warn('[AmberCallback] No pending operation found - storage may have been cleared'); - - // Check if result looks like a pubkey (login callback) - if (pubkeyRegex.test(result)) { - console.log('[AmberCallback] Result looks like pubkey, treating as login callback'); - setDebugInfo(t('settingsUi.amberCallback.validPubkey')); - try { - login.amber(result.toLowerCase()); - setDebugInfo(t('settingsUi.amberCallback.loginSuccessful')); - toast({ - title: t('settingsUi.amberCallback.loggedInTitle'), - description: t('settingsUi.amberCallback.amberLoginDesc'), - }); - // Clear debug after success - setTimeout(() => setDebugInfo(null), 2000); - return; - } catch (error) { - console.error('[AmberCallback] Amber login failed:', error); - setDebugInfo(t('settingsUi.amberCallback.loginErrorDebug', { error: String(error) })); - toast({ - title: t('settingsUi.amberCallback.loginFailedTitle'), - description: t('settingsUi.amberCallback.loginFailedDesc'), - variant: 'destructive', - }); - return; - } - } - - // Not a pubkey - can't determine what operation this was for - setDebugInfo(t('settingsUi.amberCallback.invalidResultDebug', { result: result.substring(0, 20) })); + console.warn('[AmberCallback] No pending operation found — ignoring callback'); + setDebugInfo(t('settingsUi.amberCallback.sessionLostToast')); toast({ title: 'Amber', description: t('settingsUi.amberCallback.sessionLostToast'), diff --git a/src/hooks/useDirectMessages.ts b/src/hooks/useDirectMessages.ts index 86e984f..c778b56 100644 --- a/src/hooks/useDirectMessages.ts +++ b/src/hooks/useDirectMessages.ts @@ -1,7 +1,7 @@ import { useCallback } from 'react'; import { useNostr } from '@nostrify/react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; -import { nip44, nip19, generateSecretKey, finalizeEvent, getEventHash } from 'nostr-tools'; +import { nip44, nip19, generateSecretKey, finalizeEvent, getEventHash, verifyEvent } from 'nostr-tools'; import type { NostrEvent } from '@nostrify/nostrify'; import { useNostrLogin } from '@nostrify/react/login'; @@ -493,6 +493,10 @@ export function useConversation(otherPubkey: string) { // Verify seal is kind 13 if (seal.kind !== SEAL_KIND) continue; + // SECURITY: the seal (kind 13) is the ONLY authentication for a NIP-17 + // message — the rumor (kind 14) is intentionally unsigned. Verify the + // seal's signature so a crafted seal can't impersonate another sender. + if (!verifyEvent(seal)) continue; // Decrypt seal to get rumor const rumorJson = await user.signer.nip44.decrypt(seal.pubkey, seal.content); @@ -598,6 +602,8 @@ export function useConversations() { const seal = JSON.parse(sealJson) as NostrEvent; if (seal.kind !== SEAL_KIND) continue; + // SECURITY: verify the seal's signature (see note above). + if (!verifyEvent(seal)) continue; // Decrypt seal const rumorJson = await user.signer.nip44.decrypt(seal.pubkey, seal.content); @@ -718,6 +724,7 @@ export function useGroupConversations() { const sealJson = await user.signer.nip44.decrypt(giftWrap.pubkey, giftWrap.content); const seal = JSON.parse(sealJson) as NostrEvent; if (seal.kind !== SEAL_KIND) continue; + if (!verifyEvent(seal)) continue; const rumorJson = await user.signer.nip44.decrypt(seal.pubkey, seal.content); const rumor = JSON.parse(rumorJson) as NostrEvent; @@ -837,6 +844,7 @@ export function useGroupConversation(participants: string[]) { const seal = JSON.parse(sealJson) as NostrEvent; if (seal.kind !== SEAL_KIND) continue; + if (!verifyEvent(seal)) continue; const rumorJson = await user.signer.nip44.decrypt(seal.pubkey, seal.content); const rumor = JSON.parse(rumorJson) as NostrEvent; diff --git a/src/hooks/usePayPaymentPointer.ts b/src/hooks/usePayPaymentPointer.ts index acb7f0b..5b118fb 100644 --- a/src/hooks/usePayPaymentPointer.ts +++ b/src/hooks/usePayPaymentPointer.ts @@ -19,6 +19,7 @@ import { useWallet } from '@/hooks/useWallet'; import { useLightningAddress } from '@/hooks/useLightningAddress'; import { useCashu } from '@/hooks/wallet/useCashu'; import { buildReceipt, type PaymentReceipt } from '@/lib/receipts'; +import { assertInvoiceAmount } from '@/lib/verifyInvoice'; import type { PaymentKind } from '@/hooks/useMarketplaceOffers'; export interface PayPointerArgs { @@ -59,7 +60,16 @@ export function usePayPaymentPointer() { ); } - // 2) Pay it with the best available rail, capturing the PREIMAGE where the + // 2) SECURITY: never auto-pay an invoice whose amount doesn't match the + // agreed price. A malicious seller can attach a raw BOLT11 or return an + // inflated invoice from their own LNURL server, and an NWC/WebLN wallet + // with a spend budget would pay it silently. `assertInvoiceAmount` + // rejects malformed/amountless/mismatched invoices. + if (typeof amountSats === 'number' && amountSats > 0) { + assertInvoiceAmount(invoice, amountSats); + } + + // 3) Pay it with the best available rail, capturing the PREIMAGE where the // rail returns one — that is the verifiable receipt. let preimage: string | undefined; diff --git a/src/hooks/usePlatformFee.ts b/src/hooks/usePlatformFee.ts index ed80f4f..e531d52 100644 --- a/src/hooks/usePlatformFee.ts +++ b/src/hooks/usePlatformFee.ts @@ -89,9 +89,11 @@ export const FEE_META: Record(storageKey(type), null); + const [record, setRecord] = useLocalStorage(storageKey(type, user?.pubkey ?? 'anon'), null); // Currently selected duration tier (UI state). const [tierId, setTierId] = useState(DEFAULT_TIER_ID); diff --git a/src/pages/AmberCallbackPage.tsx b/src/pages/AmberCallbackPage.tsx index a41b488..358e52e 100644 --- a/src/pages/AmberCallbackPage.tsx +++ b/src/pages/AmberCallbackPage.tsx @@ -60,8 +60,18 @@ export default function AmberCallbackPage() { const pendingOp = await getAmberPendingOperationAsync(); const pubkeyRegex = /^[0-9a-fA-F]{64}$/; + // SECURITY: only accept a result that matches an operation this app + // actually started. Never treat a bare `?event=` URL as a login — + // that would let any link switch the user into an arbitrary account. + if (!pendingOp) { + setStatus('error'); + setMessage('Login session expired or not found. Return to SkateSpots and try again.'); + clearAmberPendingOperation(); + return; + } + // ── get_public_key (login) ────────────────────────────────────────── - const isLoginOp = !pendingOp || pendingOp.type === 'get_public_key'; + const isLoginOp = pendingOp.type === 'get_public_key'; if (isLoginOp) { if (pubkeyRegex.test(result)) { // Store pubkey under a SEPARATE key so the original tab's polling