From b76764564785874a7826378ee08b55e206bb8c67 Mon Sep 17 00:00:00 2001 From: Akpamgbo Date: Thu, 25 Jun 2026 16:56:53 +0100 Subject: [PATCH] feat(backend): ledger monitor security audit, transaction signer rate limiting, crypto verification, and SQL indexes --- ...260530000000_transaction_signer_indexes.js | 82 ++++++ backend/src/app.js | 10 + backend/src/lib/horizon-poller.js | 36 ++- backend/src/lib/ledger-monitor-security.js | 245 ++++++++++++++++++ backend/src/lib/transaction-signer.js | 195 ++++++++++++++ 5 files changed, 560 insertions(+), 8 deletions(-) create mode 100644 backend/migrations/20260530000000_transaction_signer_indexes.js create mode 100644 backend/src/lib/ledger-monitor-security.js create mode 100644 backend/src/lib/transaction-signer.js diff --git a/backend/migrations/20260530000000_transaction_signer_indexes.js b/backend/migrations/20260530000000_transaction_signer_indexes.js new file mode 100644 index 00000000..f99a01ea --- /dev/null +++ b/backend/migrations/20260530000000_transaction_signer_indexes.js @@ -0,0 +1,82 @@ +/** + * Add performance indexes for Transaction Signer and payment query optimization. + * Issue #914 — Optimize SQL queries in Transaction Signer + * + * Promotes the raw SQL from backend/sql/migrations/20260529_transaction_signer_performance_indexes.sql + * into a tracked knex migration so indexes are applied automatically on deployment. + */ + +export async function up(knex) { + // Composite index for merchant payments queries + // Covers: getMerchantPayments, getRollingMetrics in paymentService.js + await knex.raw(` + CREATE INDEX CONCURRENTLY IF NOT EXISTS payments_merchant_deleted_created_idx + ON payments(merchant_id, deleted_at, created_at DESC) + `); + + // Partial index for pending payment polling in Ledger Monitor + // Covers: pollPendingPayments in horizon-poller.js + await knex.raw(` + CREATE INDEX CONCURRENTLY IF NOT EXISTS payments_status_deleted_created_idx + ON payments(status, deleted_at, created_at ASC) + WHERE status = 'pending' + `); + + // Composite index for payment lookups with soft delete + // Covers: getPaymentStatus, verifyPayment in paymentService.js + await knex.raw(` + CREATE INDEX CONCURRENTLY IF NOT EXISTS payments_id_deleted_idx + ON payments(id, deleted_at) + `); + + // Partial index for confirmation updates — optimistic locking on unclaimed rows + // Covers: checkPayment atomic update in horizon-poller.js + await knex.raw(` + CREATE INDEX CONCURRENTLY IF NOT EXISTS payments_status_txid_idx + ON payments(status, tx_id) + WHERE status = 'pending' AND tx_id IS NULL + `); + + // Composite index for merchant status queries + await knex.raw(` + CREATE INDEX CONCURRENTLY IF NOT EXISTS payments_merchant_status_created_idx + ON payments(merchant_id, status, created_at DESC) + `); + + // Composite index for recipient-based payment matching + // Covers: findMatchingPayment, findAnyRecentPayment in stellar.js + await knex.raw(` + CREATE INDEX CONCURRENTLY IF NOT EXISTS payments_recipient_asset_created_idx + ON payments(recipient, asset, created_at DESC) + WHERE deleted_at IS NULL + `); + + // Unique index on tx_id — database-level guarantee against duplicate confirmations + await knex.raw(` + CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS payments_tx_id_unique_idx + ON payments(tx_id) + WHERE tx_id IS NOT NULL + `); + + await knex.raw("ANALYZE payments"); + + console.log("✓ Added transaction signer and payment query optimization indexes"); +} + +export async function down(knex) { + const indexes = [ + "payments_merchant_deleted_created_idx", + "payments_status_deleted_created_idx", + "payments_id_deleted_idx", + "payments_status_txid_idx", + "payments_merchant_status_created_idx", + "payments_recipient_asset_created_idx", + "payments_tx_id_unique_idx", + ]; + + for (const idx of indexes) { + await knex.raw(`DROP INDEX CONCURRENTLY IF EXISTS ${idx}`); + } + + console.log("✓ Removed transaction signer optimization indexes"); +} diff --git a/backend/src/app.js b/backend/src/app.js index c626efac..a22d3cd7 100644 --- a/backend/src/app.js +++ b/backend/src/app.js @@ -37,6 +37,10 @@ import { createSep10VerifyRateLimit, createDashboardMetricsRateLimit, } from "./lib/rate-limit.js"; +import { + createTransactionSignerMiddlewares, + handleVerifySignature, +} from "./lib/transaction-signer.js"; import { versionDeprecationMiddleware } from "./lib/version-deprecation.js"; export async function createApp({ redisClient }) { @@ -312,6 +316,12 @@ export async function createApp({ redisClient }) { app.use("/api", webhooksRouter); app.use("/api/payments", paymentDetailsRouter); // NEW — GET /api/payments/:id + // Transaction Signer — rate-limited signature verification endpoint (#912) + const transactionSignerMiddlewares = createTransactionSignerMiddlewares({ + redisClient: redisAvailable ? redisClient : undefined, + }); + app.post("/api/verify-signature", ...transactionSignerMiddlewares, handleVerifySignature); + // SEP-0001 stellar.toml endpoint (public, no auth required) app.use("/", sep0001Router); diff --git a/backend/src/lib/horizon-poller.js b/backend/src/lib/horizon-poller.js index 6c1af65a..303f6b58 100644 --- a/backend/src/lib/horizon-poller.js +++ b/backend/src/lib/horizon-poller.js @@ -33,6 +33,12 @@ import { findAnyRecentPayment, verifyTransactionSignature, } from "./stellar.js"; +import { + validatePaymentRecord, + sanitizePaymentMetadata, + auditPaymentAnomaly, + isValidTransactionHash, +} from "./ledger-monitor-security.js"; import { sendWebhook, isEventSubscribed } from "./webhooks.js"; import { sendReceiptEmail } from "./email.js"; import { renderReceiptEmail } from "./email-templates.js"; @@ -304,16 +310,20 @@ async function pollPendingPayments() { async function checkPayment(payment, { merchantConfigCache = new Map() } = {}) { try { - // Guard: skip if essential fields are missing - if (!payment.asset || !payment.recipient) { + // Guard: full security validation on the DB record before touching Horizon + const validation = validatePaymentRecord(payment); + if (!validation.valid) { logger.warn( - { paymentId: payment.id }, - "Horizon poller: skipping payment with missing asset or recipient", + { paymentId: payment?.id, reason: validation.reason }, + "Horizon poller: payment record failed security validation — skipping", ); ledgerMonitorPaymentsChecked.inc({ result: "skipped" }); return; } + // Emit audit event for any anomalous field values + auditPaymentAnomaly(payment); + let match; try { match = await withLedgerMonitorRateLimit( @@ -384,13 +394,13 @@ async function checkPayment(payment, { merchantConfigCache = new Map() } = {}) { () => supabase.from("payments").update({ status: "failed", tx_id: anyPayment.transaction_hash, - metadata: { + metadata: sanitizePaymentMetadata({ ...(payment.metadata || {}), failure_reason: "underpayment", expected_amount: expected, received_amount: received, shortfall: Number((expected - received).toFixed(7)), - }, + }), }).eq("id", payment.id).eq("status", "pending"), { paymentId: payment.id, operation: "markUnderpaymentFailed" }, ); @@ -424,13 +434,13 @@ async function checkPayment(payment, { merchantConfigCache = new Map() } = {}) { status: "confirmed", tx_id: anyPayment.transaction_hash, completion_duration_seconds: Math.floor(latencySeconds), - metadata: { + metadata: sanitizePaymentMetadata({ ...(payment.metadata || {}), overpayment: true, expected_amount: expected, received_amount: received, excess: Number((received - expected).toFixed(7)), - }, + }), }).eq("id", payment.id).eq("status", "pending").is("tx_id", null).select("id").maybeSingle(), { paymentId: payment.id, operation: "confirmOverpayment" }, ); @@ -460,6 +470,16 @@ async function checkPayment(payment, { merchantConfigCache = new Map() } = {}) { return; } + // Guard: validate the transaction hash returned from Horizon before using it + if (!isValidTransactionHash(match.transaction_hash)) { + logger.warn( + { paymentId: payment.id, txHash: match.transaction_hash }, + "Horizon poller: Horizon returned an invalid transaction hash — skipping", + ); + ledgerMonitorPaymentsChecked.inc({ result: "skipped" }); + return; + } + // Guard: ensure this tx_hash hasn't already confirmed a different payment const { data: existing } = await supabase .from("payments") diff --git a/backend/src/lib/ledger-monitor-security.js b/backend/src/lib/ledger-monitor-security.js new file mode 100644 index 00000000..9fe443b5 --- /dev/null +++ b/backend/src/lib/ledger-monitor-security.js @@ -0,0 +1,245 @@ +/** + * Ledger Monitor Security Hardening — Issue #911 + * + * Implements security controls for the Ledger Monitor (horizon-poller.js): + * - Payment record validation before processing + * - Metadata sanitization to prevent injection via DB-sourced fields + * - Anomaly detection for suspicious payment patterns + * - Structured audit events for security-relevant observations + */ + +import { logger } from "./logger.js"; + +// ── Constants ───────────────────────────────────────────────────────────────── + +const STELLAR_ADDRESS_REGEX = /^G[A-Z2-7]{55}$/; +const STELLAR_TX_HASH_REGEX = /^[a-f0-9]{64}$/i; +const ASSET_CODE_REGEX = /^[A-Z0-9]{1,12}$/; + +/** Maximum byte length for a Stellar text memo. */ +const MAX_MEMO_TEXT_BYTES = 28; + +/** Maximum number of keys allowed in payment metadata to prevent resource exhaustion. */ +const MAX_METADATA_KEYS = 30; + +/** Maximum character length for any single metadata string value. */ +const MAX_METADATA_VALUE_LENGTH = 500; + +/** Keys from payment metadata that are allowed through sanitization unchanged. */ +const METADATA_ALLOWLIST = new Set([ + "order_id", + "customer_id", + "reference", + "invoice_id", + "external_id", + "failure_reason", + "expected_amount", + "received_amount", + "shortfall", + "excess", + "overpayment", + "note", +]); + +// ── Payment Record Validation ───────────────────────────────────────────────── + +/** + * Validate a payment record fetched from the database before processing. + * + * Returns `{ valid: true }` when all fields pass, or + * `{ valid: false, reason: string }` when validation fails. + * + * @param {object} payment + * @returns {{ valid: boolean, reason?: string }} + */ +export function validatePaymentRecord(payment) { + if (!payment || typeof payment !== "object") { + return { valid: false, reason: "payment record is null or not an object" }; + } + + // id + if (typeof payment.id !== "string" || payment.id.trim() === "") { + return { valid: false, reason: "payment.id is missing or not a string" }; + } + + // recipient — must be a valid Stellar public key + if (typeof payment.recipient !== "string" || !STELLAR_ADDRESS_REGEX.test(payment.recipient)) { + return { + valid: false, + reason: `payment.recipient is not a valid Stellar address: ${String(payment.recipient).slice(0, 20)}`, + }; + } + + // amount — must be a finite positive number + const amount = Number(payment.amount); + if (!Number.isFinite(amount) || amount <= 0) { + return { + valid: false, + reason: `payment.amount is not a positive finite number: ${payment.amount}`, + }; + } + + // asset — must be a valid asset code or "native" + if ( + typeof payment.asset !== "string" || + (payment.asset !== "native" && !ASSET_CODE_REGEX.test(payment.asset)) + ) { + return { + valid: false, + reason: `payment.asset is not a valid asset code: ${String(payment.asset).slice(0, 20)}`, + }; + } + + // asset_issuer — required for non-native assets, must be a Stellar address + if (payment.asset !== "native") { + if ( + typeof payment.asset_issuer !== "string" || + !STELLAR_ADDRESS_REGEX.test(payment.asset_issuer) + ) { + return { + valid: false, + reason: `payment.asset_issuer is invalid for non-native asset: ${String(payment.asset_issuer).slice(0, 20)}`, + }; + } + } + + // memo — if present, must be a string within byte limits + if (payment.memo !== null && payment.memo !== undefined) { + if (typeof payment.memo !== "string") { + return { valid: false, reason: "payment.memo is not a string" }; + } + const memoBytes = Buffer.byteLength(payment.memo, "utf8"); + if (memoBytes > MAX_MEMO_TEXT_BYTES * 4) { + // generous upper bound; Stellar SDK enforces exact limit at signing time + return { + valid: false, + reason: `payment.memo exceeds maximum byte length: ${memoBytes}`, + }; + } + } + + // created_at — must be a parseable date string + if (payment.created_at) { + const ts = Date.parse(payment.created_at); + if (!Number.isFinite(ts)) { + return { + valid: false, + reason: `payment.created_at is not a valid date: ${payment.created_at}`, + }; + } + // Reject payments with a creation time more than 1 hour in the future + if (ts > Date.now() + 60 * 60 * 1000) { + return { + valid: false, + reason: `payment.created_at is suspiciously far in the future: ${payment.created_at}`, + }; + } + } + + return { valid: true }; +} + +// ── Metadata Sanitization ───────────────────────────────────────────────────── + +/** + * Sanitize payment metadata before merging into DB update payloads. + * + * - Drops keys not on the allowlist + * - Truncates excessively long string values + * - Caps total key count + * - Removes any nested objects (flat map only) + * + * @param {unknown} metadata + * @returns {Record} + */ +export function sanitizePaymentMetadata(metadata) { + if (metadata === null || metadata === undefined) return {}; + if (typeof metadata !== "object" || Array.isArray(metadata)) return {}; + + const result = {}; + let keyCount = 0; + + for (const [key, value] of Object.entries(metadata)) { + if (keyCount >= MAX_METADATA_KEYS) break; + + if (!METADATA_ALLOWLIST.has(key)) continue; + + if (value === null || value === undefined) { + result[key] = null; + } else if (typeof value === "boolean" || typeof value === "number") { + result[key] = value; + } else if (typeof value === "string") { + result[key] = value.slice(0, MAX_METADATA_VALUE_LENGTH); + } else { + // Drop nested objects and arrays + continue; + } + + keyCount += 1; + } + + return result; +} + +// ── Anomaly Detection ───────────────────────────────────────────────────────── + +/** + * Detect anomalous patterns in a payment record that warrant a security log event. + * This does not block processing — it only emits structured warning events. + * + * @param {object} payment + */ +export function auditPaymentAnomaly(payment) { + const flags = []; + + // Unusually large amount + const amount = Number(payment.amount); + if (amount > 100_000) { + flags.push({ type: "large_amount", amount }); + } + + // Memo contains control characters or looks like an injection attempt + if (typeof payment.memo === "string") { + if (/[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/.test(payment.memo)) { + flags.push({ type: "memo_control_chars", memoLength: payment.memo.length }); + } + if (payment.memo.includes("'") || payment.memo.includes('"') || payment.memo.includes("--")) { + flags.push({ type: "memo_sql_chars" }); + } + } + + // Payment is very old (should have been handled already) + if (payment.created_at) { + const ageHours = (Date.now() - Date.parse(payment.created_at)) / 3_600_000; + if (ageHours > 20) { + flags.push({ type: "stale_payment", ageHours: Math.floor(ageHours) }); + } + } + + // Metadata has unexpected keys (keys outside the allowlist survived validation) + if (payment.metadata && typeof payment.metadata === "object") { + const unknownKeys = Object.keys(payment.metadata).filter( + (k) => !METADATA_ALLOWLIST.has(k), + ); + if (unknownKeys.length > 0) { + flags.push({ type: "metadata_unknown_keys", keys: unknownKeys.slice(0, 5) }); + } + } + + if (flags.length > 0) { + logger.warn( + { paymentId: payment.id, merchantId: payment.merchant_id, flags }, + "Ledger Monitor security: anomalous payment pattern detected", + ); + } +} + +/** + * Validate a transaction hash returned from Horizon before using it. + * + * @param {unknown} txHash + * @returns {boolean} + */ +export function isValidTransactionHash(txHash) { + return typeof txHash === "string" && STELLAR_TX_HASH_REGEX.test(txHash); +} diff --git a/backend/src/lib/transaction-signer.js b/backend/src/lib/transaction-signer.js new file mode 100644 index 00000000..6f566de1 --- /dev/null +++ b/backend/src/lib/transaction-signer.js @@ -0,0 +1,195 @@ +/** + * Transaction Signer — Issues #912 (rate limiting) and #913 (crypto signature verification) + * + * Provides a hardened wrapper around the core `verifyTransactionSignature` function + * from stellar.js with: + * + * - In-process replay attack prevention: a verified txHash is cached and any + * second attempt within the TTL window is rejected immediately. + * - XDR / txHash format validation before touching the network. + * - Rate-limit middleware factory wired to `transaction-signer-rate-limit.js`. + * - Structured audit logging on every verification outcome. + */ + +import { createHash } from "node:crypto"; +import { verifyTransactionSignature } from "./stellar.js"; +import { + createTransactionSignerRateLimit, + createTransactionSignerBurstRateLimit, + createTransactionSignerRedisStore, +} from "./transaction-signer-rate-limit.js"; +import { logger } from "./logger.js"; + +// ── Constants ───────────────────────────────────────────────────────────────── + +/** Regex for a valid Stellar transaction hash (64 hex chars). */ +const TX_HASH_REGEX = /^[a-f0-9]{64}$/i; + +/** + * How long a verified txHash is retained in the replay cache (ms). + * Must be longer than the maximum expected Horizon finality window. + */ +const REPLAY_CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes + +/** Maximum number of entries in the in-process replay cache. */ +const REPLAY_CACHE_MAX_SIZE = 10_000; + +// ── Replay Cache ────────────────────────────────────────────────────────────── + +/** + * In-process cache of recently seen txHash values that have already been + * verified. Keyed by txHash → { verifiedAt: number }. + * + * This supplements (not replaces) the DB-level unique constraint on tx_id. + * It catches replay attempts that arrive before the DB write completes. + */ +const _replayCache = new Map(); + +function pruneReplayCache() { + const now = Date.now(); + for (const [hash, entry] of _replayCache) { + if (now - entry.verifiedAt > REPLAY_CACHE_TTL_MS) { + _replayCache.delete(hash); + } + } +} + +function recordVerifiedHash(txHash) { + if (_replayCache.size >= REPLAY_CACHE_MAX_SIZE) { + // Evict oldest entry when cap is reached + const oldest = _replayCache.keys().next().value; + _replayCache.delete(oldest); + } + _replayCache.set(txHash, { verifiedAt: Date.now() }); +} + +/** Exposed for tests only. */ +export function clearReplayCache() { + _replayCache.clear(); +} + +// ── Input Validation ────────────────────────────────────────────────────────── + +/** + * Validate a transaction hash string before sending it to Horizon. + * + * @param {unknown} txHash + * @returns {{ valid: boolean, reason?: string }} + */ +export function validateTxHash(txHash) { + if (typeof txHash !== "string" || txHash.trim() === "") { + return { valid: false, reason: "txHash must be a non-empty string" }; + } + if (!TX_HASH_REGEX.test(txHash)) { + return { valid: false, reason: "txHash must be 64 lowercase hex characters" }; + } + return { valid: true }; +} + +// ── Hardened verifyTransactionSignature ────────────────────────────────────── + +/** + * Verify a Stellar transaction's cryptographic signature with replay protection. + * + * @param {string} txHash - 64-char hex transaction hash + * @param {object} [options] - Forwarded to the underlying verifyTransactionSignature + * @returns {Promise<{ valid: boolean, reason?: string, replay?: boolean, [key: string]: unknown }>} + */ +export async function verifyTransactionSignatureSecure(txHash, options = {}) { + // 1. Format validation + const formatCheck = validateTxHash(txHash); + if (!formatCheck.valid) { + logger.warn({ txHash: String(txHash).slice(0, 10), reason: formatCheck.reason }, + "TransactionSigner: invalid txHash format rejected"); + return { valid: false, reason: formatCheck.reason }; + } + + const normalizedHash = txHash.toLowerCase(); + + // 2. Replay detection — prune stale entries first + pruneReplayCache(); + if (_replayCache.has(normalizedHash)) { + logger.warn({ txHash: normalizedHash }, + "TransactionSigner: replay attempt detected — txHash already verified"); + return { valid: false, reason: "replay: txHash was already verified", replay: true }; + } + + // 3. Delegate to the core verifier + let result; + try { + result = await verifyTransactionSignature(normalizedHash, options); + } catch (err) { + logger.warn({ err, txHash: normalizedHash }, + "TransactionSigner: unexpected error during signature verification"); + return { valid: false, reason: "verification error: " + (err?.message ?? "unknown") }; + } + + // 4. Record in replay cache on success + if (result?.valid) { + recordVerifiedHash(normalizedHash); + logger.info({ + txHash: normalizedHash, + isMultiSig: result.isMultiSig, + signatureCount: result.signatureCount, + }, "TransactionSigner: signature verified successfully"); + } else { + logger.warn({ + txHash: normalizedHash, + reason: result?.reason ?? "unknown", + }, "TransactionSigner: signature verification failed"); + } + + return result ?? { valid: false, reason: "verifier returned no result" }; +} + +// ── Express Middleware Factory ───────────────────────────────────────────────── + +/** + * Build and return an array of Express middlewares for the transaction signer + * endpoint: burst limiter first, then standard limiter. + * + * @param {object} [options] + * @param {import('ioredis').Redis} [options.redisClient] - Redis client for distributed limiting + * @returns {import('express').RequestHandler[]} + */ +export function createTransactionSignerMiddlewares({ redisClient } = {}) { + let store; + if (redisClient) { + try { + store = createTransactionSignerRedisStore({ client: redisClient }); + } catch (err) { + logger.warn({ err }, "TransactionSigner: failed to create Redis store, using memory store"); + } + } + + return [ + createTransactionSignerBurstRateLimit({ store }), + createTransactionSignerRateLimit({ store }), + ]; +} + +/** + * Express route handler for POST /api/verify-signature. + * + * Expects JSON body: { txHash: string } + * Returns: { valid: boolean, reason?: string, ... } + * + * @param {import('express').Request} req + * @param {import('express').Response} res + */ +export async function handleVerifySignature(req, res) { + const txHash = req.body?.txHash ?? req.query?.txHash; + + const formatCheck = validateTxHash(txHash); + if (!formatCheck.valid) { + return res.status(400).json({ error: formatCheck.reason }); + } + + try { + const result = await verifyTransactionSignatureSecure(txHash); + return res.status(result.valid ? 200 : 422).json(result); + } catch (err) { + logger.warn({ err }, "TransactionSigner route: unhandled error"); + return res.status(500).json({ error: "Internal server error" }); + } +}