vr-shopxo-plugin/reviews/code-review-SecurityEnginee...

402 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# vr-shopxo-plugin 安全审计报告
> 审计人SecurityEngineer
> 日期2026-04-15
> 范围EventListener.php / TicketService.php / BaseService.php / ticket_detail.html / Verification.php / Ticket.php / Goods.php / plugin.json
> 评级说明:🔴 严重 / 🟡 中等 / 🟢 轻微 / 💡 建议
---
## 一、执行摘要
vr-shopxo-plugin 是一个在 ShopXO 基础上扩展票务功能的插件核心功能包括座位模板管理、电子票生成、QR 码核销、观演人信息收集。经过全面审计,共发现**1 个严重漏洞、5 个中等风险、3 个轻微问题、4 项改进建议**。最关键的问题是支付回调 `onOrderPaid` 存在并发竞态条件,可导致一张票被多次核销。
---
## 二、插件架构审计
### 2.1 生命周期钩子EventListener.php
| 钩子 | 实现 | 评估 |
|------|------|------|
| `vr_ticket_install` | ✅ | 建表语句规范,使用 `IF NOT EXISTS` 防止重复执行 |
| `vr_ticket_uninstall` | ✅ | 默认不删除数据(注释状态),安全 |
| `vr_ticket_upgrade` | ⚠️ | 仅有空壳,无版本迁移逻辑 |
**🟡 中等 — 升级迁移缺失**
`vr_ticket_upgrade($old_version)` 是空实现。若未来表结构变更(如新增字段),升级流程无法自动执行 ALTER TABLE。建议引入基于版本号的增量迁移表或脚本。
### 2.2 plugin.json 配置
```json
"hooks": [
"plugins_service_order_pay_success_handle_end",
"plugins_service_order_delete_success"
]
```
**💡 建议 — 缺少 `plugins_service_order_delete_success` 处理逻辑**
`plugin.json` 声明监听订单删除钩子,但代码中没有对应处理函数。若订单删除后票未同步处理,会导致票与订单状态不一致。
### 2.3 ALTER TABLE 安全风险
```php
$db->query("ALTER TABLE `{$prefix}goods` ADD COLUMN `item_type` VARCHAR(20) ...");
```
**🟢 轻微 — 直接执行 DDL 的并发问题**
ShopXO 可能在高并发场景下执行此 ALTER TABLE 时出现锁表。风险极低(仅执行一次),但建议在 install 中用事务或预先检测。
---
## 三、票务核心安全审计
### 3.1 🔴 严重 — `onOrderPaid` 并发竞态条件
**文件:** `TicketService.php:23-68`
```php
// 第 54-60 行:逐个生成票
foreach ($order_goods as $og) {
$ticket_id = self::issueTicket($order, $og);
// ...
}
```
**问题分析:**
- ShopXO 支付回调通过 HTTP 请求触发,如果用户使用多设备或重复点击支付,**同一订单可能被多次触发 `onOrderPaid`**
- 没有幂等性保护(无 order_id 锁或已处理标记)
- 结果:**同一订单商品可能生成多张电子票**(等于支付金额可入场多次)
**攻击场景:**
1. 攻击者购买一张票,支付 100 元
2. 支付成功回调触发 2 次(网络重试)
3. 系统生成 2 张票,每张都可独立入场核销
**修复建议:**
```php
// 在 issueTicket 前增加幂等检查
$existing = \Db::name(BaseService::table('tickets'))
->where('order_id', $order['id'])
->where('spec_base_id', $order_goods['spec_base_id'] ?? 0)
->count();
if ($existing > 0) {
return 0; // 已发放,跳过
}
```
### 3.2 🟡 中等 — `verifyTicket` 核销状态竞争
**文件:** `TicketService.php:138-196`
```php
// 第 148-154 行:状态检查
if ($ticket['verify_status'] == 1) {
return ['code' => -2, 'msg' => '该票已核销'];
}
// ... 中间无锁 ...
// 第 159-166 行:更新状态
\Db::name(BaseService::table('tickets'))
->where('id', $ticket['id'])
->update(['verify_status' => 1, ...]);
```
**问题分析:**
- 核销过程中检查SELECT与更新UPDATE分离
- 两个核销员同时扫描同一张票,在极短时间窗口内可能同时通过状态检查
- 修复:使用数据库行级锁或原子更新:
```php
$affected = \Db::name(BaseService::table('tickets'))
->where('id', $ticket['id'])
->where('verify_status', 0) // 乐观锁:只更新未核销票
->update([...]);
if ($affected === 0) {
return ['code' => -2, 'msg' => '该票已核销'];
}
```
### 3.3 🟡 中等 — QR 码 URL 直接暴露 ticket_code
**文件:** `TicketService.php: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) . '&size=8&level=H';
}
```
**问题分析:**
- QR 码内容直接 Base64 编码,**ticket_code 可见**
- QR 码打印在纸上后,攻击者可抄录 ticket_code 尝试重放
- 虽然 ticket_code 是 UUID v4随机但若泄露仍可被使用
**修复建议:**
- 使用加密后的 `qr_data` 而非原始 `ticket_code`
- 或使用一次性 token + HMAC 签名:
```php
$token = hash_hmac('sha256', $ticket_code, $secret);
$content = base64_encode(json_encode([
'type' => 'vr_ticket',
'token' => $token,
'exp' => time() + 3600, // 1小时有效
]));
```
### 3.4 🟡 中等 — `verifyTicket` 缺少权限验证
**文件:** `Ticket.php:110-128`(手动核销接口)
```php
public function verify()
{
$ticket_code = input('ticket_code', '', null, 'trim');
$verifier_id = input('verifier_id', 0, 'intval');
// ...
$result = \app\plugins\vr_ticket\service\TicketService::verifyTicket($ticket_code, $verifier_id);
```
**问题分析:**
- 接口未验证当前登录用户是否为有效核销员
- 任何登录用户(包括普通买家)只要知道 ticket_code 就可以调用此接口为他人核销
- `verifier_id` 来自前端 POST 参数,可被伪造
**修复建议:**
```php
$verifier_id = input('verifier_id', 0, 'intval');
$current_user_id = session('?user_id') ? session('user_id') : 0;
// 验证当前用户是否为注册核销员
$verifier = \Db::name(BaseService::table('verifiers'))
->where('user_id', $current_user_id)
->where('status', 1)
->find();
if (empty($verifier)) {
return DataReturn('无权核销', -1);
}
$verifier_id = $verifier['id']; // 不接受前端传入的 verifier_id
```
### 3.5 🟢 轻微 — `decryptQrData` 无法验证完整性
**文件:** `BaseService.php:68-93`
**问题分析:**
- AES-256-CBC 不提供认证加密(无 GMAC/GMAC 模式)
- 若密文被篡改解密可能成功但数据异常padding oracle 攻击理论上可能)
- ShopXO 环境风险较低(攻击者难以接触密文),但最佳实践应使用 AES-GCM
**修复建议:** 切换到 `AES-256-GCM`,或追加 HMAC-SHA256 验证。
---
## 四、前端安全审计ticket_detail.html
### 4.1 🟡 中等 — XSS 风险
**文件:** `ticket_detail.html:125`
```html
<div class="vr-event-subtitle">{$goods.simple_desc|default=''|raw}</div>
```
**问题分析:**
- `|raw` 过滤器**禁用所有 HTML 转义**,直接输出 `simple_desc`
- 若管理员在商品副标题中注入恶意脚本(如 `<img src=x onerror=alert(1)>`),所有访问该商品页的用户都会执行
- 同样风险存在于 `$goods.title`(第 124 行,无 `|raw` 安全)
**修复建议:**
- 移除 `|raw`,或对 `simple_desc` 使用白名单过滤:
```html
<div class="vr-event-subtitle">{$goods.simple_desc|default=''|htmlspecialchars}</div>
```
### 4.2 🟢 轻微 — 已选座位标签 XSS 风险
**文件:** `ticket_detail.html:275`
```html
'data-label="'+rowLabel+(colIndex+1)+'排'+(colIndex+1)+'座" '
```
**问题分析:**
- `seatInfo.label` 来自后端 JSON理论上可信但如果 seat_map JSON 数据被污染,可导致 DOM XSS
- 风险较低(需要插件管理员配合注入恶意数据)
### 4.3 🟢 轻微 — 观演人表单无服务端验证
**文件:** `ticket_detail.html:352-368`
**问题分析:**
- 观演人表单(姓名、手机、身份证)仅有 HTML5 `required` 属性
- 无服务端校验:攻击者可在提交前修改 DOM 移除 `required`,或直接 POST 构造请求上传恶意数据
- 身份证格式、手机号格式均无后端校验
### 4.4 💡 建议 — `loadSoldSeats` 未实现
**文件:** `ticket_detail.html:370-378`
```javascript
loadSoldSeats: function() {
// TODO: 从后端加载已售座位
```
已售座位数据完全依赖 JS 内存标记(空对象),**后端未实时同步座位状态**。用户可能在前端看到可选择的座位,但提交时被告知已售(库存超卖)。需实现后端座位锁定 API。
### 4.5 💡 建议 — 座位选中数量无硬上限
用户可选任意数量座位,理论上可购买全场的票。无单次购买上限限制。
---
## 五、数据库安全审计
### 5.1 SQL 注入评估
**评估结果:✅ 未发现 SQL 注入**
代码全面使用 ShopXO ORM`\Db::name()->where()->find()/select()`),所有用户输入均通过框架参数化查询:
- `Ticket.php:21``where('ticket_code|verifier_name', 'like', "%{$keywords}%")`
- `Verification.php:29-36` — 日期范围 `strtotime()` 处理
- `BaseService.php:139``where('category_id', $goods['category_id'])`
框架自动处理参数化,无 SQL 拼接风险。
### 5.2 索引评估
**✅ 索引设计合理**
- `vr_tickets` 表:`(ticket_code)` 唯一索引、`(order_id)`、`(user_id)`、`(goods_id)`、`(verify_status)` — 覆盖所有查询场景
- `vr_verifications` 表:`(ticket_id)`、`(verifier_id)` — 合理
- `vr_seat_templates` 表:`(category_id)` 唯一索引 — 合理
### 5.3 🟡 中等 — 观演人信息明文存储
**文件:** `EventListener.php:49-51`
```sql
`real_name` VARCHAR(60)
`phone` CHAR(15)
`id_card` CHAR(18)
```
**问题分析:**
- 观演人身份证、姓名、手机号**明文存储在数据库**
- 若数据库被拖库或内部人员泄露,可导致大规模个人信息泄露(中国法律要求敏感个人信息加密存储)
- 违反《个人信息保护法》第 51 条
**修复建议:**
- 对身份证号使用 AES-256-CBC 加密存储
- 或在 `vr_tickets` 表中使用单独的加密字段:
```php
'id_card_encrypted' => encrypt($attendee['id_card'] ?? ''),
```
---
## 六、支付安全审计
### 6.1 🟡 中等 — 支付回调无签名验证
**文件:** `TicketService.php:23``onOrderPaid` 回调)
ShopXO 的 `plugins_service_order_pay_success_handle_end` 钩子由 ShopXO 内部触发,但**未提供签名或 nonce 验证机制**。若存在插件注册了同名钩子,理论上可伪造回调(风险依赖 ShopXO 框架安全性)。
**当前缓解措施:** 订单状态二次验证(`$order['pay_status'] == 1`)是正确做法。
### 6.2 💡 建议 — 退款后票状态未自动更新
**文件:** `plugin.json:24`
声明了 `plugins_service_order_delete_success` 钩子但未实现。若订单退款,票状态仍为"未核销",退款用户仍可持票入场。需监听退款成功事件并将 `verify_status` 设为 2。
---
## 七、AES QR 加密方案评估
### 7.1 加密算法选择
**✅ AES-256-CBC 基本安全**
- IV 使用 `random_bytes(16)` 随机生成 ✅
- 密钥从环境变量读取 ✅
- 过期时间 `exp` 字段防止长期泄露 ✅
### 7.2 🟡 中等 — 密钥回退到硬编码默认值
**文件:** `BaseService.php:106`
```php
return config('shopxo.app_key', 'shopxo_default_secret_change_me');
```
**问题分析:**
- 若环境变量 `VR_TICKET_QR_SECRET` 未配置,且 ShopXO `app_key` 未修改默认值
- 所有 QR 密文使用同一已知密钥加密,**攻击者可离线解密所有 QR 数据**
- `shopxo_default_secret_change_me` 极可能出现在生产环境中
**修复建议:**
```php
$secret = env('VR_TICKET_QR_SECRET', '');
if (empty($secret)) {
throw new \Exception('VR_TICKET_QR_SECRET 环境变量未配置');
}
```
---
## 八、问题汇总
| 编号 | 严重程度 | 类别 | 文件 | 描述 |
|------|---------|------|------|------|
| **S-01** | 🔴 严重 | 业务逻辑 | TicketService.php:54-60 | `onOrderPaid` 无幂等保护,同一订单可生成多张票 |
| **M-01** | 🟡 中等 | 业务逻辑 | TicketService.php:138-166 | `verifyTicket` 存在 TOCTOU 竞态条件 |
| **M-02** | 🟡 中等 | 鉴权 | Ticket.php:110-128 | 手动核销接口未验证核销员身份,可被任意登录用户调用 |
| **M-03** | 🟡 中等 | 数据安全 | EventListener.php:49-51 | 观演人身份证明文存储,违反个人信息保护法律 |
| **M-04** | 🟡 中等 | 前端安全 | ticket_detail.html:125 | `simple_desc` 使用 `|raw` 导致存储型 XSS |
| **M-05** | 🟡 中等 | 加密 | BaseService.php:106 | QR 加密密钥回退到硬编码默认值 |
| **L-01** | 🟢 轻微 | 前端安全 | ticket_detail.html:275 | `data-label` 属性可能含未转义数据 |
| **L-02** | 🟢 轻微 | 数据完整性 | BaseService.php:80 | AES-CBC 无认证加密,可被 padding oracle 攻击 |
| **L-03** | 🟢 轻微 | 业务逻辑 | ticket_detail.html:370-378 | `loadSoldSeats` 未实现,存在超卖风险 |
| **I-01** | 💡 建议 | 架构 | EventListener.php:127 | 升级迁移逻辑为空 |
| **I-02** | 💡 建议 | 业务逻辑 | plugin.json:24 | 退款钩子已注册但未实现 |
| **I-03** | 💡 建议 | 前端 | ticket_detail.html:384 | 购买数量无上限控制 |
| **I-04** | 💡 建议 | 前端 | ticket_detail.html:397-407 | 观演人表单无服务端格式校验 |
---
## 九、修复优先级建议
### 立即修复(上线前必须处理)
1. **S-01** — 添加 `onOrderPaid` 幂等检查(防止一张票多次入场)
2. **M-02** — 手动核销接口鉴权(防止权限绕过)
3. **M-04** — 移除 `|raw` 或使用 `htmlspecialchars`
4. **M-05** — 移除 QR 密钥硬编码回退
### 上线后尽快修复
5. **M-01**`verifyTicket` 乐观锁
6. **M-03** — 身份证字段加密存储
7. **I-02** — 实现退款后票状态更新
### 迭代修复
8. **I-01** — 升级迁移框架
9. **L-03** — 实现实时座位锁定
10. **L-02** — 升级到 AES-GCM
---
## 十、整体评价
| 维度 | 评分 | 说明 |
|------|------|------|
| SQL 注入防护 | ⭐⭐⭐⭐⭐ | 全面使用 ORM 参数化,无注入风险 |
| 加密方案 | ⭐⭐⭐ | AES-256-CBC 基础安全,但无认证加密,密钥管理有缺陷 |
| 鉴权设计 | ⭐⭐ | 核销接口鉴权严重缺失 |
| 并发安全 | ⭐ | 核心业务流程存在明显竞态条件 |
| 数据安全合规 | ⭐⭐ | 敏感个人信息明文存储 |
| 前端安全 | ⭐⭐ | 存在存储型 XSS |
**综合评级中等风险B** — 核心业务逻辑(票务核销)存在并发安全缺陷,需优先修复后才能安全上线。