vr-shopxo-plugin/reviews/SecurityEngineer-round5-rev...

7.2 KiB

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)

-- 诊断查询
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 = <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.phpfile 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):

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:

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)

// 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

$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:

<!-- ticket/detail.html line 30 -->
<div class="detail-value">{$ticket.ticket_code}</div>  <!-- Safe: auto-escaped -->

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 controllersadmin/controller/ subdirectory controllers (Venue.php, SeatTemplate.php, etc.) are now orphaned; consider removing or deprecating