From 11fa6ccfdb4fd955bb94a7d30f9f261266d26dc3 Mon Sep 17 00:00:00 2001 From: Council Date: Wed, 15 Apr 2026 09:18:34 +0800 Subject: [PATCH] =?UTF-8?q?council(draft):=20BackendArchitect=20-=20?= =?UTF-8?q?=E8=BE=93=E5=87=BA=20vr-shopxo-plugin=20=E6=9E=B6=E6=9E=84?= =?UTF-8?q?=E8=AF=84=E5=AE=A1=E6=8A=A5=E5=91=8A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 发现严重问题： - onOrderPaid() 无幂等性（并发重复发票） - verifyTicket() TOCTOU 竞态条件 - QR Secret 默认密钥硬编码 - |raw XSS 漏洞（goods.simple_desc） - 购票参数无服务端验证 Co-Authored-By: Claude Sonnet 4.6 --- reviews/code-review-BackendArchitect.md | 352 ++++++++++++++++++++++++ 1 file changed, 352 insertions(+) create mode 100644 reviews/code-review-BackendArchitect.md diff --git a/reviews/code-review-BackendArchitect.md b/reviews/code-review-BackendArchitect.md new file mode 100644 index 0000000..017984c --- /dev/null +++ b/reviews/code-review-BackendArchitect.md @@ -0,0 +1,352 @@ +# vr-shopxo-plugin 代码审议报告 + +> 审议人：BackendArchitect +> 日期：2026-04-15 +> 审议范围：vr_ticket 插件核心代码（EventListener.php、TicketService.php、BaseService.php、ticket_detail.html） +> 视角：Backend Architect / PHP / 数据库 / 架构完整性 + +--- + +## 一、插件架构（EventListener.php / plugin.json） + +### 1.1 plugin.json 生命周期钩子缺失 ⚠️ 严重 + +**问题**：plugin.json 声明了 `hooks` 数组包含两个 hook，但文件中缺少关键的 **Install/Uninstall/Enable/Disable** 生命周期钩子。 + +```json +"hooks": [ + "plugins_service_order_pay_success_handle_end", + "plugins_service_order_delete_success" +] +``` + +ShopXO 插件标准要求实现以下函数： +- `vr_ticket_install()` — ✅ 存在 +- `vr_ticket_uninstall()` — ✅ 存在（但为空实现） +- `vr_ticket_enable()` — ❌ 缺失 +- `vr_ticket_disable()` — ❌ 缺失 + +**影响**：插件启用/停用时无法执行必要的初始化或清理操作，可能导致菜单重复注册或权限残留。 + +**修复建议**： +```php +function vr_ticket_enable() +{ + // 注册菜单/权限 + return true; +} + +function vr_ticket_disable() +{ + // 清理菜单/权限状态 + return true; +} +``` + +### 1.2 ALTER TABLE 缺少兼容性检查 ⚠️ 中等 + +EventListener.php 第 100-103 行： + +```php +$cols = $db->query("SHOW COLUMNS FROM `{$prefix}goods` LIKE 'item_type'"); +if (empty($cols)) { + $db->query("ALTER TABLE `{$prefix}goods` ADD COLUMN `item_type` ..."); +} +``` + +`SHOW COLUMNS` 返回的是结果集（resource 或 PDOStatement），不是数组，直接 `empty($cols)` 判断无效。应使用 `$cols->rowCount()` 或重新查询。 + +### 1.3 Uninstall 空实现 ⚠️ 建议 + +`vr_ticket_uninstall()` 返回 `true` 但不做任何清理。虽然保留数据是合理的设计决策，但应在代码注释中明确标注「有意保留数据」，而非看起来像是未完成的占位符。 + +### 1.4 Upgrade 空实现 ⚠️ 建议 + +`vr_ticket_upgrade($old_version)` 是空实现，未来版本升级时可能遗漏数据迁移。应在首次发布时就设计好版本号比对框架。 + +--- + +## 二、票务核心（TicketService.php / BaseService.php） + +### 2.1 onOrderPaid() 重复发放风险 — 并发漏洞 ⚠️ 严重 + +`TicketService.php` 第 23-68 行，**没有**任何幂等性保护。如果以下情况发生： + +1. ShopXO 支付回调在短时间内触发两次（网络重试） +2. 多实例部署下两个进程同时处理同一订单 + +**结果**：同一订单商品生成两张票（inventory=1 的票出现多张）。 + +代码中没有任何以下保护机制： +- 事务（Transaction） +- 悲观锁（SELECT ... FOR UPDATE） +- 幂等键（已发放标志检查） + +**修复建议**：在 `onOrderPaid()` 开头增加幂等检查： +```php +$existing = \Db::name(BaseService::table('tickets')) + ->where('order_id', $order_id) + ->find(); +if (!empty($existing)) { + BaseService::log('onOrderPaid: already issued', ['order_id' => $order_id], 'info'); + return true; // 已发放，跳过 +} +``` + +### 2.2 issueTicket() 二次写入时序问题 ⚠️ 中等 + +第 96-126 行： + +```php +$ticket_id = \Db::name(...)->insertGetId([...]); // qr_data 的 id=0 + +// 更新 QR 数据中的 ticket_id +if ($ticket_id > 0) { + $qr_payload['id'] = $ticket_id; + $qr_data_updated = BaseService::encryptQrData($qr_payload); + \Db::name(...)->where('id', $ticket_id)->update(['qr_data' => $qr_data_updated]); +} +``` + +在两步之间，如果系统读取了 ticket，会得到 `id=0` 的 QR 数据（虽然 `decryptQrData` 会成功解密，但数据内容不完整）。 + +**修复建议**：使用数据库事务包裹，或在插入前生成 UUID 作为内部关联码，而非依赖插入后的自增 ID。 + +### 2.3 verifyTicket() 核销状态竞态条件 ⚠️ 严重 + +第 138-196 行，`verifyTicket` 使用「查询-判断-更新」三步模式： + +```php +$ticket = \Db::name('tickets')->where('ticket_code', $ticket_code)->find(); +if ($ticket['verify_status'] == 1) { return ... } +\Db::name('tickets')->where('id', $ticket['id'])->update(['verify_status' => 1, ...]); +``` + +在并发场景下，两个核销员可能同时通过状态检查，导致： +- 同一张票被核销两次（记录写入两次） +- 核销员 A 成功更新后，核销员 B 的 update 仍会执行（覆盖 verifier_id） + +**修复建议**：使用带条件的原子更新： +```php +$affected = \Db::name(BaseService::table('tickets')) + ->where('id', $ticket['id']) + ->where('verify_status', 0) // 原子条件 + ->update(['verify_status' => 1, 'verify_time' => $now, ...]); +if ($affected === 0) { + return ['code' => -2, 'msg' => '该票已核销']; +} +``` + +### 2.4 QR Secret 密钥管理问题 ⚠️ 严重 + +BaseService.php 第 98-107 行： + +```php +private static function getQrSecret() +{ + $secret = env('VR_TICKET_QR_SECRET', ''); + if (!empty($secret)) { + return $secret; + } + return config('shopxo.app_key', 'shopxo_default_secret_change_me'); +} +``` + +问题： +1. **硬编码默认值**：`'shopxo_default_secret_change_me'` 若被用于生产环境，QR 加密等于明文 +2. **密钥与 ShopXO 共享**：`app_key` 可能被用于多个目的，增加密钥泄露面 +3. **无密钥强度验证**：未检查密钥长度是否满足 AES-256 要求 + +**修复建议**： +```php +private static function getQrSecret() +{ + $secret = env('VR_TICKET_QR_SECRET', ''); + if (empty($secret) || strlen($secret) < 32) { + throw new \Exception('VR票务 QR 加密密钥未配置或长度不足（需要32字符）'); + } + return $secret; +} +``` + +### 2.5 AES 加密模式无 HMAC 防篡改 ⚠️ 中等 + +`encryptQrData()` 使用 `AES-256-CBC` + `OPENSSL_RAW_DATA`，但没有对密文做 HMAC 签名。在某些 padding oracle 攻击场景下，CBC 模式可能被利用。即使当前风险较低，缺乏完整性验证是设计缺陷。 + +**修复建议**：在加密后增加 HMAC： +```php +$mac = hash_hmac('sha256', $iv . $encrypted, $secret); +return base64_encode($iv . $encrypted . $mac); +``` + +### 2.6 getQrCodeUrl() 明文暴露票码 ⚠️ 中等 + +第 220-228 行： + +```php +public static function getQrCodeUrl($ticket_code) +{ + $content = base64_encode(json_encode([ + 'type' => 'vr_ticket', + 'code' => $ticket_code, // ⚠️ 未经加密的票码直接暴露 + ])); + return ROOT_URL . '?s=index/qrcode/index&content=' . urlencode($content) ... +} +``` + +QR 码扫描后，任何人都能从 URL 中提取 `ticket_code`（只需 `base64_decode`），无需破解加密。这使得： +1. **票码可枚举**：攻击者可遍历 1-N 的 UUID 尝试核销 +2. **隐私泄露**：QR 码本身不需要包含明文票码 + +**修复建议**：QR URL 只传加密后的 payload，不包含明文 ticket_code： +```php +$qr_data = BaseService::encryptQrData([ + 'code' => $ticket_code, + // 不需要 type 明文 +]); +return ROOT_URL . '?s=index/qrcode/index&content=' . urlencode($qr_data); +``` + +--- + +## 三、前端票务详情页（ticket_detail.html） + +### 3.1 XSS 漏洞 — `|raw` 过滤器 ⚠️ 严重 + +第 125 行： +```html +
{$goods.simple_desc|default=''|raw}
+``` + +`simple_desc` 字段内容由商家后台输入，直接用 `|raw` 输出到 HTML，等于允许任意 XSS 注入。攻击者只需在商品副标题输入 `` 即可窃取用户 cookie。 + +**修复建议**： +- 移除 `|raw`，使用 `|htmlspecialchars` +- 或在后端输出前统一做 XSS 过滤 + +### 3.2 购票数据无服务端验证 ⚠️ 严重 + +第 384-422 行 `submit()` 函数： + +```javascript +var checkoutUrl = this.requestUrl + '?s=index/buy/index' + + '&goods_params=' + encodeURIComponent(goodsParams); +location.href = checkoutUrl; +``` + +购票参数（座位、票价、数量）全部由 JavaScript 计算后拼接 URL，**没有任何服务端验证**。攻击者可： +1. 篡改 `price` 为 0.01，购买任意座位 +2. 绕过前端座位数量限制，超购座位 +3. 伪造 `extension_data` 中的 `seat_info` + +**修复建议**： +- 在 `submit()` 中改为 POST 请求 +- 服务端在 `plugins_service_buy_order_create_end` hook 中**重新计算价格**，不以客户端参数为准 +- 添加签名验证（HMAC of goods_params + secret） + +### 3.3 座位图渲染 XSS 风险 — 动态插入 HTML ⚠️ 中等 + +第 271-276 行： +```javascript +rowsHtml += '
'; +``` + +`seatInfo.classes` 直接拼入 HTML class 属性。虽然 class 属性本身 XSS 风险低于 innerHTML，但如果 `classes` 值包含引号或特殊字符（如 `a" onclick="evil()"`），可破坏属性边界。 + +**修复建议**：对 `seatInfo.classes` 做属性值转义： +```javascript +var safeClasses = (seatInfo.classes || '').replace(/"/g, '"'); +``` + +### 3.4 观演人表单无服务端验证 ⚠️ 中等 + +`attendeeData`（第 395-407 行）收集的姓名、手机、身份证直接存入订单扩展数据，没有任何： +- 格式验证（手机号正则、身份证号校验） +- 长度限制 +- 恶意内容过滤（`