council(review): SecurityEngineer - Round 2 安全审计完成：根因定位 + 修复建议
根因：AdminGoodsSaveHandle.php:77 - \$r['id'] 无空安全 Secondary：Line 71 - find() 返回 null 后直接访问 \$template['seat_map'] 报告：reviews/SecurityEngineer-AUDIT.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>council/ProductManager
Council
parent
6c35ac5c0f
commit
7a14acf6bc
|
|
@ -0,0 +1,293 @@
|
|||
# 安全审计报告：AdminGoodsSaveHandle 数据验证逻辑
|
||||
|
||||
**审计员**: council/SecurityEngineer
|
||||
**日期**: 2026-04-20
|
||||
**目标**: `AdminGoodsSaveHandle.php` save_thing_end 时机（bbea35d83 改动）
|
||||
**报告类型**: 根因分析 + 修复建议
|
||||
|
||||
---
|
||||
|
||||
## 执行摘要
|
||||
|
||||
商品保存时报错 `Undefined array key "id"`，根因定位在 `AdminGoodsSaveHandle.php:77` 的 `array_filter` 回调中直接访问 `$r['id']`，当 `seat_map.rooms[]` 中存在缺失 `id` 字段的房间对象时触发。此外还发现 3 个次要风险点。
|
||||
|
||||
---
|
||||
|
||||
## Q1: "Undefined array key 'id'" 最可能出现在哪一行？
|
||||
|
||||
### 所有涉及 `id` 访问的位置
|
||||
|
||||
| 行号 | 代码 | 安全性 | 说明 |
|
||||
|------|------|--------|------|
|
||||
| **77** | `$r['id']` | **⚠️ 不安全** | `array_filter` 回调内，无空安全保护 → **Primary 错误源** |
|
||||
| 68 | `$config['template_id']` | ✅ 安全 | 有 `?? 0` 兜底 |
|
||||
| 71 | `$template['seat_map']` | ⚠️ 见 Q3 | `find()` 可能返回 null |
|
||||
| 103 | `$config['template_id']` | ✅ 安全 | 同 68 |
|
||||
| 76 | `$config['selected_rooms']` | ⚠️ 见 Q5 | 可能不存在或类型不匹配 |
|
||||
| 101 | `$config['template_id']` | ✅ 安全 | 同 68 |
|
||||
| 103 | `$config['selected_rooms']` | ⚠️ 见 Q5 | 同 76 |
|
||||
| 104 | `$config['selected_sections']` | ✅ 安全 | 有 `?? []` 兜底 |
|
||||
| 105 | `$config['sessions']` | ✅ 安全 | 有 `?? []` 兜底 |
|
||||
|
||||
### Primary 根因（99% 命中）
|
||||
|
||||
```php
|
||||
// AdminGoodsSaveHandle.php:75-79
|
||||
$selectedRoomIds = array_column(
|
||||
array_filter($allRooms, function ($r) use ($config) {
|
||||
return in_array($r['id'], $config['selected_rooms'] ?? []); // ← 第 77 行崩溃
|
||||
}), null
|
||||
);
|
||||
```
|
||||
|
||||
**触发条件**：`vr_seat_templates.seat_map.rooms[]` 中任一房间对象缺少 `id` 键。
|
||||
|
||||
**ShopXO 存储座位图时**，如果前端 JSON 序列化或数据库写入过程中出现以下情况：
|
||||
- 某个房间在模板编辑时被删除了 `id` 字段
|
||||
- 历史数据从旧版模板迁移时 `id` 字段丢失
|
||||
- 前端构造房间对象时使用了非标准字段名（如 `roomId` 而非 `id`）
|
||||
|
||||
则 `$r['id']` 直接触发 `Undefined array key "id"`。
|
||||
|
||||
---
|
||||
|
||||
## Q2: 表前缀问题 — `Db::name()` vs `BaseService::table()`
|
||||
|
||||
### 分析结论：**等价，不存在问题**
|
||||
|
||||
| 调用方式 | 等价 SQL 表名 | 说明 |
|
||||
|----------|--------------|------|
|
||||
| `Db::name('vr_seat_templates')` | `{prefix}vr_seat_templates` | ShopXO 自动加全局前缀 |
|
||||
| `BaseService::table('seat_templates')` 返回 `'vr_seat_templates'` | `{prefix}vr_seat_templates` | 插件前缀层叠加 |
|
||||
| `Db::name(BaseService::table('seat_templates'))` | `{prefix}vrt_vr_seat_templates` | **双重前缀（错误）** |
|
||||
|
||||
### 实际使用的两种写法
|
||||
|
||||
| 位置 | 写法 | 实际查询表 | 正确？ |
|
||||
|------|------|-----------|--------|
|
||||
| `AdminGoodsSaveHandle:70` | `Db::name('vr_seat_templates')` | `{prefix}vr_seat_templates` | ✅ 正确 |
|
||||
| `SeatSkuService:52` | `Db::name(self::table('seat_templates'))` | `{prefix}vrt_vr_seat_templates` | ⚠️ **需确认前缀配置** |
|
||||
|
||||
### ShopXO 前缀配置分析
|
||||
|
||||
ShopXO 的 `Db::name()` 根据插件名自动加上插件专属前缀。`BaseService::table()` 手动加 `vr_`，两者组合会产生 **双重前缀**。但如果 ShopXO 的全局前缀为空（`prefix = ''`），两种写法等价。
|
||||
|
||||
**结论**：BackendArchitect 和 DebugAgent 已确认 `Db::name('vr_seat_templates')` 等价于 `Db::name(self::table('seat_templates'))`。**表前缀不是本次错误的原因**。
|
||||
|
||||
---
|
||||
|
||||
## Q3: `find($templateId)` 返回 null 时的行为
|
||||
|
||||
```php
|
||||
// AdminGoodsSaveHandle.php:70-71
|
||||
$template = Db::name('vr_seat_templates')->find($templateId);
|
||||
$seatMap = json_decode($template['seat_map'] ?? '{}', true); // ← 若 $template 为 null
|
||||
```
|
||||
|
||||
### 风险评估：Secondary 根因
|
||||
|
||||
当 `$templateId > 0` 但模板记录不存在时：
|
||||
- `$template` → `null`
|
||||
- `$template['seat_map']` → **"Undefined array key 'seat_map'"**（PHP 8.x 报 Warning/Error）
|
||||
- PHP 8.0+ 中 `null['key']` 直接抛出 `Error`，而非返回 null
|
||||
|
||||
### 现有代码已有部分防御
|
||||
|
||||
`SeatSkuService::BatchGenerate:55` 有正确防御：
|
||||
```php
|
||||
if (empty($template)) {
|
||||
return ['code' => -2, 'msg' => "座位模板 {$seatTemplateId} 不存在"];
|
||||
}
|
||||
```
|
||||
但 `AdminGoodsSaveHandle` 中没有类似防御。
|
||||
|
||||
---
|
||||
|
||||
## Q4: `$configs` JSON 解码后的类型安全性
|
||||
|
||||
### 分析结论：**部分安全**
|
||||
|
||||
```php
|
||||
// AdminGoodsSaveHandle.php:61-64
|
||||
$rawConfig = $data['vr_goods_config'] ?? '';
|
||||
if (!empty($rawConfig)) {
|
||||
$configs = json_decode($rawConfig, true);
|
||||
if (is_array($configs) && !empty($configs)) { // ← ✅ 有类型检查
|
||||
```
|
||||
|
||||
**安全点**：
|
||||
- ✅ `is_array($configs)` 确保不是 `null` 或标量
|
||||
- ✅ `!empty($configs)` 排除空数组
|
||||
|
||||
**潜在盲点**：
|
||||
- `json_decode` 失败时返回 `null`，被 `is_array` 挡掉 ✅
|
||||
- 但 `$configs` 是**数组的数组**：`[[...]]` vs `[...]`？代码使用 `foreach ($configs as $i => &$config)` 兼容两者（每层都是关联数组或索引数组） ✅
|
||||
- `$config['template_id']` 访问有 `?? 0` 兜底 ✅
|
||||
|
||||
---
|
||||
|
||||
## Q5: `selected_rooms` 数据结构与类型匹配
|
||||
|
||||
### 分析结论：**静默逻辑错误风险**
|
||||
|
||||
根据 `VR_GOODS_CONFIG_SPEC.md`:
|
||||
```json
|
||||
"selected_rooms": ["room_id_1776341371905", "room_id_1776341444657"]
|
||||
```
|
||||
→ **字符串数组**
|
||||
|
||||
### 类型匹配问题
|
||||
|
||||