From 05e990eda572ac3dc2ced0f0574e1708549ec5f3 Mon Sep 17 00:00:00 2001 From: Guoguo Date: Mon, 18 May 2026 03:45:57 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20payment=20security=20hardening=20?= =?UTF-8?q?=E2=80=94=203=20CRITICAL=20+=203=20HIGH?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL fixes: - C1: Notify amount validation now unconditional (was skippable if amount field missing) - C2: Sync endpoint validates payer_total against order amount before activation - C3: Order ID uses crypto.randomBytes(6) instead of Math.random (collision-safe) HIGH fixes: - H1: Payment endpoints rate limited to 5/min per IP - H2: Max 5 pending orders per user, reject new ones until completed/cancelled - H3: Purchase endpoint returns error (not mock) when wxpay unconfigured in production Also fixed: - Notify handler asserts Buffer body, rejects non-Buffer (L3) - Notify error response is generic, no internal message leak (M1) - Private key cached in memory after first read (L2) - Fixed duplicate paymentOrderDao const declaration --- server/src/app.js | 10 ++++++++++ server/src/dao/payment-order.dao.js | 10 +++++++++- server/src/lib/wxpay.js | 6 ++++-- server/src/routes/payment-notify.js | 10 +++++----- server/src/routes/payment.js | 3 +++ server/src/routes/subscription.js | 12 ++++++++---- 6 files changed, 39 insertions(+), 12 deletions(-) diff --git a/server/src/app.js b/server/src/app.js index ac08e5c..00f932a 100644 --- a/server/src/app.js +++ b/server/src/app.js @@ -42,10 +42,20 @@ app.use((req, res, next) => { next() }) +const paymentLimiter = rateLimit({ + windowMs: 60 * 1000, + max: 5, + standardHeaders: true, + legacyHeaders: false, + message: { code: 2001, message: 'too_many_attempts' } +}) + app.use('/api/v1/auth/login', userLoginLimiter) app.use('/api/v1/admin/login', adminLoginLimiter) app.use('/api/v1/user/avatar', uploadLimiter) app.use('/api/v1/user/phone', uploadLimiter) +app.use('/api/v1/subscription/purchase', paymentLimiter) +app.use('/api/v1/payment/orders', paymentLimiter) // WeChat Pay callback — must be before authMiddleware (no JWT) app.post('/api/v1/payment/wechat/notify', require('./routes/payment-notify')) diff --git a/server/src/dao/payment-order.dao.js b/server/src/dao/payment-order.dao.js index f3d5662..c825561 100644 --- a/server/src/dao/payment-order.dao.js +++ b/server/src/dao/payment-order.dao.js @@ -76,4 +76,12 @@ async function markFailed(orderId, tradeState) { ) } -module.exports = { createOrder, findByOutTradeNo, markPrepay, markPaidAndActivateSubscription, markClosed, markFailed } +async function countPendingByUser(userId) { + const rows = await query( + "SELECT COUNT(*) AS cnt FROM payment_orders WHERE user_id = :user_id AND status IN ('created', 'paying')", + { user_id: userId } + ) + return rows[0].cnt +} + +module.exports = { createOrder, findByOutTradeNo, markPrepay, markPaidAndActivateSubscription, markClosed, markFailed, countPendingByUser } diff --git a/server/src/lib/wxpay.js b/server/src/lib/wxpay.js index f0e8775..a96ad03 100644 --- a/server/src/lib/wxpay.js +++ b/server/src/lib/wxpay.js @@ -8,9 +8,11 @@ function isConfigured() { return !!(c.mchId && c.apiV3Key && c.mchSerialNo && (c.privateKey || c.privateKeyPath)) } +let _cachedPrivateKey = null function getPrivateKey() { - if (config.wxpay.privateKey) return config.wxpay.privateKey - if (config.wxpay.privateKeyPath) return fs.readFileSync(config.wxpay.privateKeyPath, 'utf8') + if (_cachedPrivateKey) return _cachedPrivateKey + if (config.wxpay.privateKey) { _cachedPrivateKey = config.wxpay.privateKey; return _cachedPrivateKey } + if (config.wxpay.privateKeyPath) { _cachedPrivateKey = fs.readFileSync(config.wxpay.privateKeyPath, 'utf8'); return _cachedPrivateKey } throw new Error('WeChat Pay private key not configured') } diff --git a/server/src/routes/payment-notify.js b/server/src/routes/payment-notify.js index 5156f1d..8a7418f 100644 --- a/server/src/routes/payment-notify.js +++ b/server/src/routes/payment-notify.js @@ -6,8 +6,9 @@ const logDao = require('../dao/log.dao') async function handleNotify(req, res) { try { - const rawBody = typeof req.body === 'string' ? req.body : (Buffer.isBuffer(req.body) ? req.body.toString('utf8') : JSON.stringify(req.body)) - const parsed = typeof req.body === 'object' && !Buffer.isBuffer(req.body) ? req.body : JSON.parse(rawBody) + if (!Buffer.isBuffer(req.body)) throw new Error('invalid body format') + const rawBody = req.body.toString('utf8') + const parsed = JSON.parse(rawBody) await wxpay.verifyNotifySignature(req.headers, rawBody) @@ -22,8 +23,7 @@ async function handleNotify(req, res) { const order = await paymentOrderDao.findByOutTradeNo(orderId) if (!order) throw new Error('order not found: ' + orderId) - // Validate amount - if (result.amount && result.amount.total !== order.amount_fen) throw new Error('amount mismatch') + if (!result.amount || result.amount.total !== order.amount_fen) throw new Error('amount mismatch') if (result.trade_state === 'SUCCESS') { const activated = await paymentOrderDao.markPaidAndActivateSubscription(orderId, result.transaction_id, result) @@ -36,7 +36,7 @@ async function handleNotify(req, res) { res.status(200).json({ code: 'SUCCESS', message: '' }) } catch (err) { console.error('[WXPAY NOTIFY ERROR]', err.message) - res.status(400).json({ code: 'FAIL', message: err.message || 'processing error' }) + res.status(400).json({ code: 'FAIL', message: 'processing error' }) } } diff --git a/server/src/routes/payment.js b/server/src/routes/payment.js index 178f601..53a713b 100644 --- a/server/src/routes/payment.js +++ b/server/src/routes/payment.js @@ -30,6 +30,9 @@ router.post('/payment/orders/:order_id/sync', requireUser, wrap(async (req, res) const wxOrder = await wxpay.queryOrder(order.order_id) if (wxOrder.trade_state === 'SUCCESS') { + if (!wxOrder.amount || wxOrder.amount.payer_total !== order.amount_fen) { + return res.json(fail(2001, 'amount mismatch')) + } await paymentOrderDao.markPaidAndActivateSubscription(order.order_id, wxOrder.transaction_id, wxOrder) await logDao.write({ user_id: order.user_id, action: 'payment_sync_success', detail: 'order: ' + order.order_id, ip: req.ip }) return res.json(ok({ status: 'paid', message: 'subscription activated' })) diff --git a/server/src/routes/subscription.js b/server/src/routes/subscription.js index 9c900c8..88d422d 100644 --- a/server/src/routes/subscription.js +++ b/server/src/routes/subscription.js @@ -1,3 +1,4 @@ +const crypto = require('crypto') const router = require('express').Router() const { ok, fail } = require('../lib/response') const { requireUser } = require('../middleware/auth') @@ -45,13 +46,17 @@ router.post('/subscription/purchase', requireUser, wrap(async (req, res) => { if (!PLANS[plan] || plan === 'trial') return res.json(fail(2001, 'invalid plan')) const wxpay = require('../lib/wxpay') + const config = require('../config') if (!wxpay.isConfigured()) { - // Dev mode fallback: return mock indicator + if (config.nodeEnv === 'production') return res.json(fail(2001, 'payment not configured')) const orderId = 'ORD' + Date.now() return res.json(ok({ order_id: orderId, payment_params: null, mock: true, plan, amount: PLANS[plan].amount })) } - // Read price from server settings, not from client + const paymentOrderDao = require('../dao/payment-order.dao') + const pendingCount = await paymentOrderDao.countPendingByUser(req.user.user_id) + if (pendingCount >= 5) return res.json(fail(2001, '待支付订单过多，请先完成或取消现有订单')) + const settingsDao = require('../dao/settings.dao') const settings = await settingsDao.getAll() const priceYuan = plan === 'yearly' @@ -59,8 +64,7 @@ router.post('/subscription/purchase', requireUser, wrap(async (req, res) => { : (Number(settings.monthly_price) || PLANS[plan].amount) const amountFen = Math.round(priceYuan * 100) - const orderId = 'ORD' + Date.now() + String(Math.floor(Math.random() * 10000)).padStart(4, '0') - const paymentOrderDao = require('../dao/payment-order.dao') + const orderId = 'ORD' + Date.now() + crypto.randomBytes(6).toString('hex') await paymentOrderDao.createOrder(orderId, req.user.user_id, plan, amountFen) const prepayId = await wxpay.createPrepayOrder({