From 35c10a7f66d40905ecbfc22943f21827cb1f2cfc Mon Sep 17 00:00:00 2001 From: Council Date: Thu, 16 Apr 2026 08:53:41 +0800 Subject: [PATCH] council(security): SecurityEngineer - add missing VenueList methods + security audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security findings: - SQL injection: LOW (query builder + parameter binding) - XSS: LOW (ThinkPHP auto-escape, no |raw detected) - Path traversal: LOW (all view paths hardcoded) - CSRF: MEDIUM (ShopXO framework-level gap, out of scope for plugin) Critical fix: admin/Admin.php was missing VenueList(), VenueSave(), VenueDelete() — sidebar URL "/plugins/vr_ticket/admin/venueList" would return 500 error. Added all three methods with v3.0 seat_map support. P1 garbled name: documented DB fix SQL for shx_plugins + vrt_power tables. Co-Authored-By: Claude Sonnet 4.6 --- plan.md | 39 ++++ reviews/SecurityEngineer-round5-review.md | 194 ++++++++++++++++ shopxo/app/plugins/vr_ticket/admin/Admin.php | 227 ++++++++++++++++++- 3 files changed, 459 insertions(+), 1 deletion(-) create mode 100644 reviews/SecurityEngineer-round5-review.md diff --git a/plan.md b/plan.md index b77fa2d..f591146 100644 --- a/plan.md +++ b/plan.md @@ -128,3 +128,42 @@ app/plugins/vr_ticket/ | P1-T4 | [Pending] | 需实际访问 URL 截图验证 | | P2-T1 | [Pending] | 数据库编码检查（需 DB 访问）| | P2-T2 | [Pending] | 数据库修复（如需要）| + +--- + +## SecurityEngineer Round 5 补充 + +### 关键发现：VenueList() 方法缺失（Critical Bug） +plugin.json sidebar URL `/plugins/vr_ticket/admin/venueList` 链接到 `VenueList()` 方法，但 admin/Admin.php 中该方法不存在 → 点击"场馆配置"菜单会导致 500 错误。 + +**已修复**：在 admin/Admin.php 中添加： +- `VenueList()` — 场馆列表（含 v3.0 seat_map 解析） +- `VenueSave()` — 场馆创建/编辑（含 v3.0 JSON 构建和验证） +- `VenueDelete()` — 场馆软删除（含审计日志） +- `countSeatsV2()` — v2 格式（数组）座位计数辅助方法 + +### 安全审计结论 + +| 安全项 | 风险等级 | 结论 | +|--------|----------|------| +| SQL 注入 | LOW | 所有查询使用 ThinkPHP query builder + 参数绑定 | +| XSS | LOW | ThinkPHP 模板引擎自动转义，无 `\|raw` 输出 | +| 路径遍历 | LOW | 所有视图路径为硬编码方法名，无用户输入 | +| CSRF | MEDIUM | ShopXO 框架级缺失，插件层面无法单独修复 | +| 数据编码（P1乱码）| LOW | DB latin1 存储导致乱码，非安全漏洞 | + +### P1 乱码 DB 修复 SQL + +```sql +-- 1. 诊断 +SELECT id, name, title, LENGTH(name), HEX(name) FROM shx_plugins WHERE name LIKE '%vr%'; + +-- 2. 修复 plugins 表 +UPDATE shx_plugins SET name = 'vr_ticket', title = 'VR票务' WHERE name = 'vr_ticket'; + +-- 3. 修复 vrt_power 表（如果存在乱码） +SELECT id, name, LENGTH(name), HEX(name) FROM vrt_power WHERE name LIKE '%票%'; +UPDATE vrt_power SET name = 'VR票务' WHERE HEX(name) LIKE '%E7A58A%'; +``` + +详细安全分析见：`reviews/SecurityEngineer-round5-review.md` diff --git a/reviews/SecurityEngineer-round5-review.md b/reviews/SecurityEngineer-round5-review.md new file mode 100644 index 0000000..c83b61c --- /dev/null +++ b/reviews/SecurityEngineer-round5-review.md @@ -0,0 +1,194 @@ +# SecurityEngineer Round 5 Review — vr_ticket Phase 2 Security Analysis + +> Date: 2026-04-16 | Reviewer: SecurityEngineer | Scope: admin/Admin.php + plugin.json + Vrticket.php + +--- + +## Executive Summary + +Two root causes confirmed from previous rounds: +- **P1 (乱码)**: DB field double-encoding (UTF-8 bytes stored as Latin1) +- **P2 (路由)**: Controller in wrong subdirectory; missing `VenueList()` method + +**New finding this round**: Critical missing `VenueList()` method blocks sidebar navigation. + +--- + +## P1 — Sidebar Garbled Text (`VR票务`) + +### Root Cause +The garbled `VR票务` = UTF-8 bytes decoded as Latin1: +- `票务` UTF-8 bytes: `E7 A5 8E E5 8A B1` +- Interpreted as Latin1 → `ç ¥¨ 务` +- **Source**: `plugins` DB table's `name`/`title` field (not `plugin.json` — that file is correct UTF-8) +- The sidebar menu reads plugin name from the database, not from `plugin.json` + +### Fix Required (DB operation) +```sql +-- 诊断查询 +SELECT id, name, title, LENGTH(name), HEX(name) FROM shx_plugins WHERE name LIKE '%票%'; + +-- 修复：将 latin1 乱码字段更新为正确的 UTF-8 中文 +UPDATE shx_plugins +SET name = 'vr_ticket', title = 'VR票务' +WHERE name = 'vr_ticket'; + +-- 同时检查 vrt_power 表 +SELECT id, name, LENGTH(name), HEX(name) FROM vrt_power WHERE name LIKE '%票%'; +-- 如果有乱码： +UPDATE vrt_power SET name = 'VR票务' WHERE id = ; +``` + +### Security Note +This is a **data corruption** issue, not a security vulnerability. No injection risk here. + +--- + +## P2 — Routing Failure + +### Root Cause +1. **Old controller** `admin/controller/Venue.php` uses namespace `app\plugins\vr_ticket\admin\controller\Venue` +2. ShopXO `PluginsService::PluginsControlCall` loads class `app\plugins\vr_ticket\admin\Venue` (without `controller` subdir) +3. This resolves to `app/plugins/vr_ticket/admin/Venue.php` — **file doesn't exist** +4. The new `admin/Admin.php` has the correct PSR-4 path but was **missing `VenueList()` and `VenueSave()` methods** + +### Fix Applied +Added to `admin/Admin.php`: +- `VenueList()` — delegates to `plugins_vr_seat_templates` listing with v3.0 `seat_map` parsing +- `VenueSave()` — full create/update with v3.0 JSON construction +- `VenueDelete()` — soft delete with audit logging +- `countSeatsV2()` — helper for array-format seat_map (v2) + +### URL Routing Analysis +| Sidebar URL in plugin.json | ucfirst chain | Expected method | Status | +|---|---|---|---| +| `/plugins/vr_ticket/admin/seatTemplateList` | `SeatTemplateList` | `SeatTemplateList()` | EXISTS | +| `/plugins/vr_ticket/admin/ticketList` | `TicketList` | `TicketList()` | EXISTS | +| `/plugins/vr_ticket/admin/verifierList` | `VerifierList` | `VerifierList()` | EXISTS | +| `/plugins/vr_ticket/admin/verificationList` | `VerificationList` | `VerificationList()` | EXISTS | +| `/plugins/vr_ticket/admin/venueList` | `VenueList` | `VenueList()` | **WAS MISSING — NOW ADDED** | + +--- + +## P3 — View Path Security + +### Analysis +The view paths in `Admin.php` are all **hardcoded strings** (not user input): +```php +return view('seat_template/list', [...]); // hardcoded +return view('ticket/list', [...]); // hardcoded +return view('venue/list', [...]); // hardcoded +``` + +ThinkPHP resolves these relative to the current controller's view directory (`app/plugins/vr_ticket/admin/view/`). **No path traversal risk.** + +The `Vrticket.php` (main controller) uses: +```php +return MyView('plugins/view/vr_ticket/admin/view/' . $template); +``` +Template values like `'seat_template/list'` are also hardcoded in each method. **Low risk.** + +### Security Verdict +- **Path traversal**: LOW RISK — all paths are hardcoded method names +- **Information disclosure**: LOW RISK — ThinkPHP's view resolution returns a clear error if template missing + +--- + +## SQL Injection Analysis + +### Safe Patterns (All queries use ThinkPHP query builder with parameter binding) +```php +// SAFE — column name hardcoded, value parameterized +$where[] = ['name', 'like', "%{$name}%"]; +$where[] = ['status', '=', intval($status)]; + +// SAFE — query builder with array $where +$list = \Db::name('plugins_vr_seat_templates')->where($where)->paginate(20); + +// SAFE — direct intval on foreign key +$id = input('id', 0, 'intval'); +\Db::name('...')->where('id', $id)->find(); +``` + +### Minor Concern +```php +$where[] = ['name', 'like', "%{$name}%"]; // $name comes from input('name', '', null) +// input() with null filter = no type coercion +// If $name contains special chars like % or _, they become part of the LIKE pattern +// Low risk — only affects own query results, no data exfiltration +``` +**Recommendation**: Consider escaping `%` and `_` in LIKE patterns if `$name` can be user-supplied without validation. + +### Verdict: LOW RISK +All database access uses ThinkPHP's query builder. No raw SQL. No string concatenation in queries. + +--- + +## XSS Analysis + +### ThinkPHP Template Auto-Escape +All views use `{$var.property}` syntax which auto-escapes HTML: +```html + +
{$ticket.ticket_code}
+``` + +### Potential Risk +If any view uses `{$data|raw}` or `{*variable*}` (raw output), XSS is possible. Quick scan of available templates shows all using `{$var}` safe syntax. + +### JSON API Responses +All `DataReturn()` calls return JSON — no HTML output, no XSS risk. + +### Verdict: LOW RISK +ThinkPHP template engine provides auto-escaping. No raw output detected. + +--- + +## CSRF Analysis + +### Finding: No CSRF Protection +ShopXO does not implement CSRF token validation for admin POST requests. All POST endpoints in `Admin.php` are vulnerable to CSRF: +- `SeatTemplateSave()` — create/update seat template +- `TicketVerify()` — manually verify ticket +- `VerifierSave()` — add/update verifier +- `VerifierDelete()` — disable verifier +- `VenueSave()` — create/update venue +- `VenueDelete()` — disable venue + +### Risk Assessment +**MEDIUM** — Requires authenticated admin session. An attacker could: +- Trick admin into clicking a link that deletes/verifies records +- Social engineering attack vector + +### Recommendation +ShopXO should implement CSRF middleware globally. For this plugin, the framework-level fix is needed (not plugin-level). + +### Verdict: MEDIUM (framework-level issue, not plugin-specific) + +--- + +## Summary + +| Issue | Severity | Status | +|---|---|---| +| P1: Garbled sidebar name | LOW (UX bug) | DB fix documented | +| P2: Missing VenueList() | **HIGH** (blocks navigation) | **FIXED** | +| SQL Injection | LOW | No issues found | +| XSS | LOW | Auto-escaping confirmed | +| Path Traversal | LOW | Hardcoded paths only | +| CSRF | MEDIUM (framework) | Out of scope for plugin | + +--- + +## Fixes Applied This Round + +1. **Added `VenueList()`** to `admin/Admin.php` — critical, sidebar URL was broken +2. **Added `VenueSave()`** with full v3.0 JSON construction and validation +3. **Added `VenueDelete()`** with audit logging +4. **Added `countSeatsV2()`** helper for array-format seat_map parsing + +## Still Needed + +1. **DB fix for P1** — run the SQL UPDATE to fix garbled plugin name in `shx_plugins` table +2. **Verify in browser** — confirm sidebar shows `VR票务` and all 5 menu items work +3. **Clean up old controllers** — `admin/controller/` subdirectory controllers (Venue.php, SeatTemplate.php, etc.) are now orphaned; consider removing or deprecating diff --git a/shopxo/app/plugins/vr_ticket/admin/Admin.php b/shopxo/app/plugins/vr_ticket/admin/Admin.php index 9c4a1ae..ce193cb 100644 --- a/shopxo/app/plugins/vr_ticket/admin/Admin.php +++ b/shopxo/app/plugins/vr_ticket/admin/Admin.php @@ -478,6 +478,212 @@ class Admin extends Common return DataReturn('已禁用', 0); } + // ============================================================ + // 场馆配置（Venue） + // 视图: admin/view/venue/{action}.html + // 注意：admin/controller/Venue.php 的旧控制器仍在使用， + // 但新路由走 Admin.php 的 VenueList/VenueSave。 + // ============================================================ + + /** + * 场馆列表 + * URL: /plugins/vr_ticket/admin/venueList + */ + public function VenueList() + { + $where = []; + + $name = input('name', '', null); + if ($name !== '') { + $where[] = ['name', 'like', "%{$name}%"]; + } + + $status = input('status', '', null); + if ($status !== '' && $status !== null) { + $where[] = ['status', '=', intval($status)]; + } + + $list = \Db::name('plugins_vr_seat_templates') + ->where($where) + ->order('id', 'desc') + ->paginate(20) + ->toArray(); + + // 解析 venue.name 和座位数（v3.0 格式：seat_map.venue.name） + foreach ($list['data'] as &$item) { + $seatMap = json_decode($item['seat_map'] ?? '{}', true); + $item['venue_name'] = $seatMap['venue']['name'] ?? $item['name']; + $item['venue_address'] = $seatMap['venue']['address'] ?? ''; + $item['zone_count'] = !empty($seatMap['sections']) ? count($seatMap['sections']) : 0; + $item['seat_count'] = $this->countSeatsV2($seatMap); + } + unset($item); + + return view('venue/list', [ + 'list' => $list['data'], + 'page' => $list['page'], + 'count' => $list['total'], + ]); + } + + /** + * 添加/编辑场馆 + * URL: /plugins/vr_ticket/admin/venueSave + */ + public function VenueSave() + { + $id = input('id', 0, 'intval'); + + if (IS_AJAX_POST) { + $data = [ + 'name' => input('name', '', null, 'trim'), + 'category_id' => input('category_id', 0, 'intval'), + 'status' => input('status', 1, 'intval'), + 'upd_time' => time(), + ]; + + if (empty($data['name'])) { + return DataReturn('场馆名称不能为空', -1); + } + + // v3.0 seat_map JSON 构建 + $venue = [ + 'name' => input('venue_name', '', null, 'trim'), + 'address' => input('venue_address', '', null, 'trim'), + 'image' => input('venue_image', '', null, 'trim'), + ]; + if (empty($venue['name'])) { + return DataReturn('场馆名称不能为空', -1); + } + + $zones_raw = input('zones', '[]', null, 'trim'); + $zones = json_decode($zones_raw, true); + if (!is_array($zones) || empty($zones)) { + return DataReturn('请至少添加一个分区', -1); + } + + $map_raw = input('seat_map_rows', '[]', null, 'trim'); + $map = json_decode($map_raw, true); + if (!is_array($map) || empty($map)) { + return DataReturn('座位排布不能为空', -1); + } + + $sections = []; + $seats = []; + $row_labels = []; + foreach ($zones as $zone) { + $char = strtoupper($zone['char'] ?? ''); + if (empty($char)) { + return DataReturn('分区字符不能为空', -1); + } + $sections[] = [ + 'char' => $char, + 'name' => $zone['name'] ?? '', + 'color' => $zone['color'] ?? '#cccccc', + ]; + $seats[$char] = [ + 'price' => intval($zone['price'] ?? 0), + 'color' => $zone['color'] ?? '#cccccc', + 'label' => $zone['name'] ?? '', + ]; + } + + foreach ($map as $rowStr) { + $rowStr = trim($rowStr); + if (empty($rowStr)) { + return DataReturn('座位排布每行不能为空', -1); + } + foreach (str_split($rowStr) as $char) { + if ($char !== '_' && !isset($seats[$char])) { + return DataReturn("座位排布中字符 '{$char}' 未在分区中定义", -1); + } + } + $row_labels[] = $rowStr[0]; + } + + $data['seat_map'] = json_encode([ + 'venue' => $venue, + 'map' => $map, + 'seats' => $seats, + 'sections' => $sections, + 'row_labels' => array_values(array_unique(array_merge( + array_column($sections, 'char'), + $row_labels + ))), + ], JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); + + if ($id > 0) { + \Db::name('plugins_vr_seat_templates')->where('id', $id)->update($data); + return DataReturn('更新成功', 0); + } else { + $data['add_time'] = time(); + $data['upd_time'] = time(); + $data['spec_base_id_map'] = ''; + \Db::name('plugins_vr_seat_templates')->insert($data); + return DataReturn('添加成功', 0); + } + } + + $info = []; + if ($id > 0) { + $row = \Db::name('plugins_vr_seat_templates')->find($id); + if (!empty($row)) { + $seatMap = json_decode($row['seat_map'] ?? '{}', true); + $row['venue_name'] = $seatMap['venue']['name'] ?? ''; + $row['venue_address'] = $seatMap['venue']['address'] ?? ''; + $row['venue_image'] = $seatMap['venue']['image'] ?? ''; + $row['zones_json'] = json_encode($seatMap['sections'] ?? [], JSON_UNESCAPED_UNICODE); + $row['venue_json'] = json_encode([ + 'name' => $seatMap['venue']['name'] ?? '', + 'address' => $seatMap['venue']['address'] ?? '', + 'image' => $seatMap['venue']['image'] ?? '', + ], JSON_UNESCAPED_UNICODE); + $row['map_json'] = json_encode($seatMap['map'] ?? [], JSON_UNESCAPED_UNICODE); + $info = $row; + } + } + + $categories = \Db::name('GoodsCategory') + ->where('is_delete', 0) + ->order('id', 'asc') + ->select(); + + return view('venue/save', [ + 'info' => $info, + 'categories' => $categories, + ]); + } + + /** + * 删除场馆（软删除） + */ + public function VenueDelete() + { + if (!IS_AJAX_POST) { + return DataReturn('非法请求', -1); + } + + $id = input('id', 0, 'intval'); + if ($id <= 0) { + return DataReturn('参数错误', -1); + } + + $template = \Db::name('plugins_vr_seat_templates')->where('id', $id)->find(); + \Db::name('plugins_vr_seat_templates') + ->where('id', $id) + ->update(['status' => 0, 'upd_time' => time()]); + + \app\plugins\vr_ticket\service\AuditService::log( + \app\plugins\vr_ticket\service\AuditService::ACTION_DISABLE_TEMPLATE, + \app\plugins\vr_ticket\service\AuditService::TARGET_TEMPLATE, + $id, + ['before_status' => $template['status'] ?? 1], + $template ? "场馆: {$template['name']}" : "ID:{$id}" + ); + + return DataReturn('删除成功', 0); + } + // ============================================================ // 核销记录（Verification） // 视图: admin/view/verification/{action}.html @@ -564,7 +770,7 @@ class Admin extends Common // ============================================================ /** - * 统计座位数（来自原 SeatTemplate） + * 统计座位数（v1 格式：直接传入 seat_map JSON 字符串） */ private function countSeats($seat_map_json) { @@ -585,4 +791,23 @@ class Admin extends Common } return $count; } + + /** + * 统计座位数（v2 格式：直接传入已解码的 seat_map 数组） + */ + private function countSeatsV2(array $seat_map) + { + if (empty($seat_map['seats']) || empty($seat_map['map'])) { + return 0; + } + $count = 0; + foreach ($seat_map['map'] as $row) { + foreach (str_split($row) as $char) { + if ($char !== '_' && isset($seat_map['seats'][$char])) { + $count++; + } + } + } + return $count; + } }