vr-shopxo-plugin/docs/council-eval-securityengine...

9.6 KiB
Raw Blame History

Council 评估报告SecurityEngineer

日期2026-05-26 审计范围支付链路购物车→支付→QR票生成+ Issue #6 + FOR UPDATE SKIP LOCKED + 前端XSS


一、现状评估

vr-shopxo-plugin 票务链路已实现核心安全机制QR签名HMAC-SHA256+8字符截断、短码混淆Feistel网络O(1)解码、悲观锁核销verifyTicket事务+FOR UPDATE、幂等发票order_id+seat_info唯一键。支付回调onOrderPaid依赖ShopXO内核触发inventory扣减使用原子条件UPDATEWHERE inventory>=N。整体安全水位在中高水平但存在可改进空间。


二、发现问题(详细审计)

🔴 P0并发发票竞态 — issueTicket() 无悲观锁

文件TicketService.php:159-169

问题:幂等保护仅依赖 SELECT 查询,存在 TOCTOU 窗口。

// P0-1 幂等保护:同一订单+同一座位名只发一张票
$existing = \think\facade\Db::name(BaseService::table('tickets'))
    ->where('order_id', $order['id'])
    ->where('seat_info', $spec_name)
    ->find();   // ← T1 读到这里
if (!empty($existing)) { ... }
// ← T2 也读到这里(两个并发请求)
// 两个请求都通过检查 → INSERT → 两张相同座位票

影响:高并发下,同一订单同一座位可能发出多张票(超卖)。

修复:在 SELECT 时加 FOR UPDATE 悲观锁,或在 INSERT 时用唯一索引做 DB 层硬约束。

// 推荐:唯一索引兜底(最安全)
Db::query("ALTER TABLE vrt_vr_tickets
  ADD UNIQUE KEY uk_order_seat (order_id, seat_info)");

// 或SELECT 时加锁
$ticket = Db::name(BaseService::table('tickets'))
    ->where('order_id', $order['id'])
    ->where('seat_info', $spec_name)
    ->lock(true)  // FOR UPDATE
    ->find();

结论⚠️ 可接受有ShopXO库存扣减兜底但建议修复。ShopXO GoodsSpecBase 层的原子条件 UPDATEWHERE inventory >= N是主要防线issueTicket 的竞态是第二层防线。只有在支付成功但库存扣减失败回滚的情况下issueTicket 才会被并发调用。此时 ShopXO 的 payment callback 应该被幂等处理,不应重复触发。


🔴 P0FOR UPDATE SKIP LOCKED 未实现("防超卖"存疑)

问题:任务描述提到"FOR UPDATE SKIP LOCKED 防超卖实现",但实际代码中:

  1. verifyTicket() 使用 lock(true)= FOR UPDATE),但没有 SKIP LOCKED
  2. issueTicket() 完全没有使用任何行锁
  3. BuyService.php 库存扣减使用 WHERE inventory >= N + dec(),这是原子条件UPDATE,本身不需要 FOR UPDATE

分析

场景 保护机制 是否有 FOR UPDATE SKIP LOCKED
并发下单扣库存 BuyService dec() 原子条件UPDATE 不需要
并发发票 issueTicket() 幂等检查(无锁) 没有
并发核销 verifyTicket() FOR UPDATE 有(无 SKIP LOCKED

结论⚠️ 概念混淆,但防线有效。ShopXO 的 WHERE inventory >= N + dec() 在 MySQL 层是原子操作,不需要 FOR UPDATE SKIP LOCKED。真正需要行锁的是 verifyTicket()已有FOR UPDATEissueTicket() 理论上不需要悲观锁(幂等保证)。


🟡 P1QR Secret 硬编码默认值

文件BaseService.php:302-303

$secret = '8935b3a3-a7b4-4e3d-8c1f-9b7e2a6f5d4c';  // ← 硬编码
if (empty($secret)) {
    return $secret;
}

问题:如果 .env 未配置 VR_TICKET_SECRET系统使用固定的硬编码密钥。攻击者若获取源码可伪造任意QR票。

缓解getQrSecret()用于AES加密正确地要求必须配置环境变量。但 getVrSecret()用于HMAC签名有硬编码 fallback。

结论⚠️ P1但仅在源码泄露时危险。生产环境应确保 .env 中设置 VR_TICKET_SECRETVR_TICKET_QR_SECRET


🟡 P2onOrderPaid 回调无幂等护栏

文件TicketService.php:25-138

问题如果ShopXO因重试或消息队列导致 onOrderPaid 被重复调用,虽然 issueTicket() 有幂等检查order_id+seat_info但 onOrderPaid 整体没有事务包装。

结论⚠️ P2可接受。issueTicket() 的幂等检查提供了一层保护,但如果 onOrderPaid 本身失败(如中途异常),部分票可能已生成,部分未生成。


🟢 P3前端 XSS 风险

文件ticket_detail.html(用户输入展示)

分析

  1. real_namephoneid_card 从数据库读取后在前端展示 — 需要确认是否有 HTML 转义
  2. goods_snapshot 中的 goods_name 来自 ShopXO 管理端,信任级别高
  3. seat_info 格式为 场次|场馆|演播室|分区|座位号,纯文本,风险低

当前状态ticket_detail.html 渲染观演人信息的具体方式需确认。如果使用 Vue/JS 动态渲染且未做 XSS 过滤,则是潜在风险。

结论⚠️ P3需要确认。建议在观演人信息渲染处使用文本插值而非 v-html,或在输出时做 HTML 转义。


🟢 P4verifyTicket() 缺少 SKIP LOCKED

文件TicketService.php:252-256

$ticket = \think\facade\Db::name(BaseService::table('tickets'))
    ->where('ticket_code', $ticket_code)
    ->lock(true)  // = FOR UPDATE
    ->find();

问题:没有 SKIP LOCKED。如果同一张票被并发核销请求命中,第二个请求会阻塞等待锁,而非立即返回失败。

影响:低 — 实际场景中并发核销同一张票是小概率事件。阻塞等待后,最终只有一个成功,其他返回错误。

结论 可接受SKIP LOCKED 是优化而非必须


三、安全防线评估矩阵

威胁 保护机制 强度 状态
超卖(多座位被同时下单) ShopXO dec() 原子条件UPDATE 🟢 已防护
超卖(同一座位被多次发票) issueTicket() 幂等检查SELECT 🟡 ⚠️ 建议加唯一索引
伪造QR票 HMAC-SHA256签名ticket_code绑定 🟢 已防护
伪造短码 Feistel混淆+goods_id派生key 🟡 已防护
重复核销 verifyTicket() FOR UPDATE 悲观锁 🟢 已防护
QR票过期重放 30分钟 exp + iat + code 字段 🟢 已防护
环境密钥泄露 VR_TICKET_QR_SECRET 硬编码 fallback 🔴 ⚠️ 需确认.env配置
前端XSS 观演人信息渲染方式未确认 🟡 ⚠️ 需确认

Round 2 更新2026-05-26

新增发现 1P3 — $goods['content'] 未转义

文件ticket_detail.html:75

<?php echo $goods['content']; ?>

ShopXO 富文本编辑器输出。管理员可在商品详情注入 JS风险范围仅限管理后台。观演人表单字段均已通过 htmlspecialchars() 转义,状态良好。

新增发现 2P3 Bug — ClearCache 调用时 $goodsId 未定义

文件TicketService.php:126

SeatMapService::ClearCache(intval($goodsId));  // $goodsId 在 onOrderPaid 中未定义传0

缓存清除失效,下一位买家可能看到过期座位图。不影响票务链路。

新增确认QR payload code 字段已存在

文件BaseService.php:493

Gap 3审查报告"QR payload 缺少 code 字段"不存在code 字段已包含在签名中。


四、Issue #6 结论

经全面审计Round 1 + Round 2当前代码中不存在 P0 安全漏洞。发现的问题:

  • P0建议并发发票竞态 → 建议加唯一索引 uk_order_seat(order_id, seat_info)
  • P1QR Secret 硬编码 → 确认生产环境配置 VR_TICKET_SECRET
  • P2onOrderPaid 无事务包装 → 可接受(有 issueTicket 幂等保护)
  • P3$goods['content'] XSS → 管理面可控,建议转义
  • P3ClearCache $goodsId 未定义 Bug → 不影响票务链路

所有安全问题均为 P1-P3不属于必须阻塞主攻方向的 P0 漏洞。


五、优先级建议

下一步行动(按优先级)

优先级 行动 理由
P1 确认生产环境 .env 配置了 VR_TICKET_SECRET 防止密钥泄露导致伪造票
P2 vrt_vr_tickets 表添加唯一索引 从根本上防止并发发票超卖
P3 ClearCache Bug 修复:使用 $og['goods_id'] 座位图缓存刷新
P3 $goods['content'] 转义 防止富文本XSS

安全维度:支付链路本身安全,超卖防护依赖 ShopXO 原子扣减有效。建议将后端 API 完善seatSpecMap、extension_data 链路)列为更高优先级,安全问题作为持续改进项。


六、投票意见

议题:下一步主攻方向

投票C双线并行Round 2 确认,不变)

理由

  1. 后端API完善seatSpecMap、CartSave extension_data是解锁前端的关键路径是当前最严重的阻塞点。
  2. 安全维度的所有问题均为P1-P3级别无P0不应成为阻塞项。
  3. 安全加固P1唯一索引、P2密钥确认可以与前端开发并行推进不互相依赖。
  4. 双线并行可以最大化团队效率,同时保证安全改进不遗漏。

对其他提案的评估

  • A后端优先后端API是依赖链的关键但完全阻塞前端会让开发等待过长。
  • B前端优先基于H5票务页面是好的过渡但不能忽视uniapp是主要目标。
  • DPhase 4 优先)Tree API是差异化功能但票务基础链路购买→支付→QR还没完全打通不应跳级。