本文由 FlintHub 开发文档归档发布。原始文档日期见标题前缀，来源：7月5日复制的资料/代码审查计划与方法论.md。

SoSite 代码审查计划与方法论

创建日期：2026-07-02

审查方法：按安全风险 + 架构核心顺序，而非目录顺序

一、审查优先级

第一优先级 — 架构核心 + 安全入口（先审）

第二优先级 — 安全支撑层

第三优先级 — 业务 Controller

第四优先级 — 其余文件

Model / View / Plugin / Theme — 一般不影响全局安全，但可能存在逻辑 Bug。

二、审查方法论

每个文件按以下维度逐项检查：

结论格式

项目        结果
安全性      ⭐⭐⭐⭐⭐
架构        ⭐⭐⭐⭐☆
可维护性    ⭐⭐⭐⭐☆
性能        ⭐⭐⭐⭐⭐
风险等级    低 / 中 / 高 / 严重

问题列表格式

## 问题 N：标题
- **类型**：Bug / 安全 / 架构 / 性能 / 可维护性
- **风险**：低 / 中 / 高 / 严重
- **位置**：文件名:行号
- **原因**：
- **建议**：
- **修复代码**：（必要时提供）

三、审查记录

文件 1：app/Helpers/Permission.php

审查结论

整体评价

Permission.php 整体设计合理，核心思路清晰——管理员组（group_id=4）跳过所有检查、白名单限制、参数化查询。大部分已知隐患已在 2026-07-01/02 的安全修复中处理。但仍存在以下问题：

问题 1：check() 每次查数据库获取 group_id（性能 + 一致性）

• 类型：性能 / 架构
• 风险：低
• 位置：Permission.php:164-168

$groupId = 0;
if ($userId !== null && $userId > 0) {
    $db = \app\Core\Database::getInstance();
    $row = $db->fetchOne('SELECT group_id FROM users WHERE id = :id', [':id' => $userId]);
    $groupId = (int)($row['group_id'] ?? 0);
}

原因：每次权限检查都从数据库查询 group_id。一个页面加载可能调用 canView() 多次（首页帖子列表），产生 N+1 查询。Session 中已存有 user_id 和 role，但未缓存 group_id。

建议：让调用方直接传入 $groupId 作为参数，或使用 Auth::getCurrentUser()['group_id']（已有静态缓存）：

public static function canView(int $categoryId, ?int $userId = null): bool
{
    // 从 Auth 获取 group_id（带静态缓存）
    $groupId = 0;
    if ($userId !== null && $userId > 0) {
        $user = \app\Helpers\Auth::getCurrentUser();
        $groupId = $user ? (int)($user['group_id'] ?? 0) : 0;
    }
    return self::checkByGroup('can_view', $categoryId, $groupId);
}

但注意：Auth::getCurrentUser() 也查数据库（但有静态缓存），改不改取决于性能敏感度。当前规模下影响不大。

问题 2：getAuthorizedCategoryIds() 逻辑分支嵌套深，容易遗漏

• 类型：可维护性
• 风险：低
• 位置：Permission.php:87-140

原因：该方法有 5 个条件分支（管理员、有记录直接返回、无记录全量、游客回退、该组无记录全量），逻辑瀑布较长。新增一个分支时容易破坏现有逻辑。

建议：不影响功能，暂不改动。如需重构，可拆为：

• getAllCategoryIds() — 查全部版块
• getGroupCategoryIds($groupId, $perm) — 查某组有权限的版块
• getFallbackForGuest() — 游客回退逻辑

问题 3：check() 中游客回退与 getAuthorizedCategoryIds() 中游客回退逻辑不完全一致

• 类型：Bug / 逻辑一致性
• 风险：中
• 位置：Permission.php:186-193 vs Permission.php:120-127

check() 中的游客回退（按版块级别）：

if ($groupId === 0) {
    $fallbackRow = $db->fetchOne(
        "SELECT {$perm} FROM category_permissions WHERE category_id = :cid AND group_id = 1",
        [':cid' => $categoryId]
    );
    if ($fallbackRow) {
        return (int)$fallbackRow[$perm] === 1;
    }
}

getAuthorizedCategoryIds() 中的游客回退：

if ($groupId === 0) {
    $fallbackRows = $db->fetchAll(
        'SELECT category_id FROM category_permissions WHERE group_id = 1 AND can_view = 1'
    );
    $fallbackIds = array_map('intval', array_column($fallbackRows, 'category_id'));
    return $fallbackIds;
}

风险：check() 在找不到 group=1 的记录时会进入"全版块默认允许"逻辑（第 196-206 行）。而 getAuthorizedCategoryIds() 在游客回退找不到 group=1 的记录时直接返回空数组，不回退到全版块。

场景：管理员创建了新版块，尚未配置任何权限记录：

• check() → 走到第 199 行 SELECT COUNT(*) FROM category_permissions WHERE category_id = :cid → 返回 0 → return true（允许）
• getAuthorizedCategoryIds() → 走到第 120-127 行 → $fallbackIds 为空 → return []（不显示）

结果：首页帖子列表看不到新板块的帖子（getAuthorizedCategoryIds() 返回空 → getLatestAllowed([]) 返回空），但直接访问 /forum/category/{id} 时 check() 却允许查看。两个入口行为不一致。

建议：在 getAuthorizedCategoryIds() 的游客回退之后增加兜底：

if ($groupId === 0) {
    $fallbackIds = array_map('intval', array_column(/* group=1 的 can_view */));
    if (!empty($fallbackIds)) return $fallbackIds;
    // 兜底：如果 group=1 也没有权限记录，返回所有版块（兼容未配置状态）
    $all = $db->fetchAll('SELECT id FROM categories');
    return array_map('intval', array_column($all, 'id'));
}

问题 4：check() 缺少 $categoryId <= 0 的边界检查

• 类型：Bug
• 风险：中
• 位置：Permission.php:154-207

原因：如果调用方传入 $categoryId = 0 或负数，SQL 查询 WHERE category_id = 0 AND group_id = :gid 不会报错，但逻辑上无意义。可能导致意外放行或拒绝。

建议：

private static function check(string $perm, int $categoryId, ?int $userId = null): bool
{
    if ($categoryId <= 0) return false;
    // ...
}

问题 5：saveGroup() 没有对内置组（id 1~4）的写入限制

• 类型：安全
• 风险：低
• 位置：Permission.php:223-233

原因：saveGroup() 允许直接更新 id 1~4 内置组的任何属性，包括 is_default。虽然 GroupController 中在保存前做了限制（不允许内置组设为默认组），但 saveGroup() 作为公开静态方法，如果未来被其他代码调用可能绕过。

建议：在 saveGroup() 内部也加一道保险：

public static function saveGroup(int $id, string $name, string $color, int $isDefault): void
{
    if ($id > 0 && $id <= 4) {
        $isDefault = 0; // 内置组不允许设为默认组
    }
    // ... 原有逻辑
}

问题 6：saveCategoryPermission() 中 ON DUPLICATE KEY UPDATE 使用了 category_id + group_id 的唯一键

• 类型：安全
• 风险：低
• 位置：Permission.php:262-274

原因：$canView、$canPost、$canReply 从调用方传入，但已通过 (int) 强制转换，无注入风险。使用了 ON DUPLICATE KEY UPDATE，并发安全。

评价：该实现正确且安全，无需改动。

文件 2：app/Helpers/Auth.php

审查结论

整体评价

Auth 本身代码质量不差（getCurrentUser() 请求级缓存设计很好，requireAdmin() 二次验证逻辑清晰），但权限模型存在严重分裂——Auth 以 role 判定管理员，Permission 以 group_id=4 判定管理员，两套体系各自为政，这是整个项目目前最大的架构缺陷。

问题 1：isAdmin() 与 Permission 使用两套权限模型（严重）

• 类型：安全 / 架构
• 风险：🔴 严重
• 位置：Auth.php:12 vs Permission.php:54

// Auth.php
public static function isAdmin(): bool
{
    return isset($_SESSION['role']) && $_SESSION['role'] === 'admin';
}

// Permission.php
public static function isAdminGroup(int $userId): bool
{
    return self::getUserGroupId($userId) === 4;
}

后果：role=admin + group_id=1 → 进得了后台但论坛权限全无；role=user + group_id=4 → 论坛全部放行但进不了后台。两个管理员体系互相矛盾。

建议：统一判定，至少同时校验 role 和 group_id：

public static function isAdmin(): bool
{
    $user = self::getCurrentUser();
    return $user
        && $user['role'] === 'admin'
        && (int)($user['group_id'] ?? 1) === 4;
}

问题 2：isAdmin() 完全信任 Session（高风险）

• 类型：安全
• 风险：🔴 高
• 位置：Auth.php:12-14

原因：$_SESSION['role'] 直接决定后台权限。如果管理员被后台降权，旧 Session 仍保持管理员身份。数据库修改角色后 Session 不会自动同步。

建议：以数据库作为最终可信来源，Session 只保存 user_id，isAdmin() 改为从 getCurrentUser() 的数据库结果判断。

问题 3：用户已删除但 Session 仍存活（中风险）

• 类型：Bug
• 风险：🟠 中
• 位置：getCurrentUser() 方法

原因：如果 user_id 在数据库中已不存在（被删除、数据库异常），getCurrentUser() 返回 null，但 Session 仍保留 user_id 和 role。系统进入"已登录但无用户"的异常状态。

建议：

$user = $db->fetchOne(...);
if (!$user) {
    $_SESSION = [];
    session_destroy();
    return null;
}

问题 4：requireAdmin() 已登录用户跳登录页（低风险）

• 类型：体验
• 风险：🟡 低
• 位置：requireAdmin() 方法

原因：非管理员访问后台时直接跳到 /login，但用户可能已经登录（只是不是管理员）。应该区分"未登录→登录页"和"已登录但无权限→403"。

建议：

if (!self::isLoggedIn()) {
    header('Location: /login');
} elseif (!self::isAdmin()) {
    http_response_code(403);
    echo '权限不足';
}
exit;

问题 5：用户缓存依赖手工失效（低风险）

• 类型：可维护性
• 风险：🟡 低
• 位置：getCurrentUser() 静态缓存

原因：clearUserCache() 依赖人工调用，用户更新操作中容易忘记调用，导致同一请求内读到旧数据。

建议：将用户更新操作统一封装，自动调用缓存失效。

问题 6：Session 保存了过多信息

• 类型：架构
• 风险：🟢 低
• 位置：Session 写入处

原因：当前 Session 保存了 user_id、role，与数据库可能不同步。建议仅保存 user_id 和 group_id（性能优化用），其余全部从数据库读取。

问题 7：getCurrentUser() 的缓存设计是亮点

• 类型：✅ 值得保留
• 位置：getCurrentUser()

static $initialized = false;
static $cached = null;

请求级缓存，20 次调用只查一次数据库，设计良好。

问题 8：requireAdmin() 的二次验证逻辑清晰

• 类型：✅ 值得保留
• 位置：requireAdmin()

admin_verified_at 超时判断和 admin_redirect_after_verify 来源保存都设计合理。

文件 3：app/Core/Controller.php

审查结论

整体评价

Controller 基类"足够薄"，职责单一，没有混入业务逻辑。公共逻辑（安全 Header、CSRF Token 注入、RememberMe、当前用户）集中管理。主要问题集中在与 Auth 的联动和请求初始化时机上。

问题 1：RememberMe::check() 放在 view() 中而非 init.php（高风险）

• 类型：架构 / Bug
• 风险：🔴 高
• 位置：Controller.php — view() 方法

原因：只有渲染页面才恢复登录。API、AJAX、json()、redirect() 都不会触发 RememberMe 恢复。用户明明有 Cookie，但 /api/vote 等接口仍然未登录。

建议：将 RememberMe::check() 移到 app/init.php，确保整个请求统一恢复。

问题 2：updateOnlineStatus() 同样位置不当（高风险）

• 类型：架构 / Bug
• 风险：🔴 高
• 位置：Controller.php — view() 方法

原因：只有渲染页面才会更新在线状态，AJAX、发帖、回复、上传等请求不会更新，导致在线用户统计不准确。

建议：移到 init.php 或 Auth 初始化流程中。

问题 3：requireLogin() 仅检查 Session 而非数据库状态（中风险）

• 类型：安全
• 风险：🟠 中
• 位置：requireLogin()

原因：只检查 Auth::isLoggedIn()（即 Session 中是否存在 user_id），不确认数据库中用户是否仍存在且状态正常。用户被删除后 Session 仍认为已登录。

建议：

public function requireLogin(): void
{
    $user = Auth::getCurrentUser();
    if (!$user) {
        Auth::logout();
        $this->redirect('/login');
        exit;
    }
}

问题 4：redirect() 对非 HTTP 协议未处理（低风险）

• 类型：安全
• 风险：🟡 低
• 位置：redirect()

原因：当前检查了 strpos($url, 'http') === 0 时校验域名，但 javascript:alert(1) 以 javascript: 开头，不会进校验。现代浏览器 Location 不执行 javascript:，但建议白名单化。

建议：仅允许以 / 开头的相对路径：

if (!str_starts_with($url, '/')) {
    $url = '/';
}

问题 5：viewRaw() 没有走公共初始化流程（低风险）

• 类型：可维护性
• 风险：🟡 低
• 位置：viewRaw()

原因：view() 会执行 RememberMe、安全 Header、公共变量注入、Plugin Hook，而 viewRaw() 直接 display。以后两者行为不一致容易出问题。

建议：抽取公共方法如 prepareView()，view() 和 viewRaw() 都调用。

问题 6：json() 缺少 JSON_THROW_ON_ERROR（建议）

• 类型：健壮性
• 风险：🟢 低
• 位置：json()

建议：

json_encode($data, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES | JSON_THROW_ON_ERROR)

问题 7：CSP 和 Permissions-Policy 安全头缺失（建议）

• 类型：增强
• 风险：🟢 低

建议：增加 Content-Security-Policy 和 Permissions-Policy 响应头。

问题 8：Plugin Hook 无异常保护（建议）

• 类型：健壮性
• 风险：🟢 低

建议：

try {
    Plugin::hook('controller_view_before', ['data' => &$data]);
} catch (\Throwable $e) {
    \error_log('Plugin hook error: ' . $e->getMessage());
}

四、阶段性总结（截至 Controller.php 审毕）

当前重点问题已从"代码缺陷"转向"架构设计"层面：

五、修复优先级总表（截至当前）

六、审查者建议

1. 权限检查中间件化（长期建议）：当前每个 Controller 方法手动调用 Permission::canView()，容易遗漏。未来可在 Controller 基类中加 $requirePermission 属性数组，自动在 dispatch 时注入检查。

1. check() 与 getAuthorizedCategoryIds() 的游客回退一致性（短期建议）：建议统一两处的回退逻辑，确保游客在同一版块上两个入口判决一致。

1. getAuthorizedCategoryIds() 的首屏性能（数据量大后关注）：每次加载首页时查询 user_groups、category_permissions、categories 等多个表，建议加一层静态缓存（请求级别），避免同一请求中多次调用的重复查询。

1. 区分 userId=0 和 groupId=0：当前两套体系在多个方法中混合，建议新增 checkByGroup() 方法，让 Controller 直接传 groupId，消除混淆。

文件 4 & 5：app/Controllers/ThreadController.php + PostController.php

审查结论

整体评价

代码结构非常爽朗清晰，标准的 MVC 模式。但一旦进入业务层，系统设计的缺陷（尤其是权限和边界）就立刻变成实实在在的安全漏洞。本次审计以实战攻防级别进行，发现两个严重 IDOR 漏洞。

🔴 漏洞一：全站任意附件删除（严重 IDOR）

• 类型：安全（IDOR）
• 风险：🔴 严重
• 位置：ThreadController::edit() — delete_attachments 处理

问题：删除附件时没有校验附件是否属于当前帖子或当前用户：

foreach ($_POST['delete_attachments'] as $attId) {
    \app\Helpers\Settings::deleteAttachment((int)$attId);
}

攻击场景：恶意用户 F12 修改 delete_attachments[] 为 1,2,3,4,5...，即可删掉全站所有人所有附件。

修复：

$attachment = $db->fetchOne("SELECT * FROM attachments WHERE id = :id", [':id' => $attId]);
if ($attachment && (int)$attachment['thread_id'] === $threadId) {
    \app\Helpers\Settings::deleteAttachment((int)$attId);
}

🔴 漏洞二：跨版块任意移帖与越权发帖（高危逻辑漏洞）

• 类型：安全（权限绕过）
• 风险：🔴 严重
• 位置：ThreadController::edit() — category_id 修改

问题：编辑帖子时允许修改 category_id（移动版块），但完全没有对目标版块进行 canPost 权限校验：

$categoryId = (int)($_POST['category_id'] ?? 0);
$threadModel->update($threadId, ['category_id' => $categoryId, ...]);

攻击场景：用户在公开版块发帖 → 编辑 → category_id 改成 VIP 版块 ID → 越权偷渡帖子。

修复：

if ($categoryId !== (int)$thread['category_id']) {
    if (!\app\Helpers\Permission::canPost($categoryId, (int)$currentUser['id'])) {
        $error = '您没有权限将帖子移动到该版块';
    }
}

🟠 漏洞三：普通用户篡改主题高亮颜色（功能越权）

• 类型：安全（越权）
• 风险：🟠 中
• 位置：ThreadController::edit() — color 字段

问题：直接信任 $_POST['color']，普通用户可绕过前端给自己帖子染高亮色。

$color = trim($_POST['color'] ?? '');

修复：

$color = $this->isAdmin() ? trim($_POST['color'] ?? '') : $thread['color'];

🟠 问题四：回复接口缺乏事务保护

• 类型：数据一致性
• 风险：🟠 中
• 位置：ThreadController::show() POST 分支

问题：同时做了 Post::insert、incrementReplyCount、Points::award、Settings::runtimeIncr、多文件循环上传 + INSERT attachments，但没有事务保护（PostController::create 里有事务）。附件上传中途失败会产生半死不活的不一致状态。

修复：将 POST 内所有写操作包裹在 $db->begin() / commit() / rollback() 中。

🟡 问题五：requireLogin 错位

• 类型：逻辑
• 风险：🟡 低
• 位置：ThreadController::show() POST 分支

问题：requireLogin() 放在了 if ($content !== '') 分支内部，游客发空 POST 可绕过身份校验（虽然不写数据但不合设计预期）。

修复：POST 入口立即执行 requireLogin() 和权限判定，再校验表单数据。

文件 8：app/Helpers/Upload.php

审查结论

整体评价

应用层防护做得几乎无懈可击（白名单 in_array(..., true)、无条件 finfo MIME 校验、uniqid 重命名、空字节截断防御）。安全隐患主要集中在底层第三方依赖库（Imagick/GD）对恶意文件的解析上。

🔴 漏洞一：Imagick 远程代码执行（RCE 风险）

• 类型：安全（RCE）
• 风险：🔴 严重
• 位置：Upload::makeThumbnailImagick()

问题：ImageMagick 历史上爆发过多次严重 RCE（ImageTragick、MSL/SVG/MVG 解析）。代码虽限制了后缀，但 Imagick 不信任后缀，会解析文件内容头决定解码器。黑客上传 .jpg 但内含恶意 SVG 脚本 → new \Imagick($sourcePath) → 命令执行。

修复：显式限制 Imagick 允许的解码器：

$img = new \Imagick();
$img->setRegistry('URL', '');
$img->setOption('svg:auto-graphic', 'false');
$img->readImage($sourcePath);

🔴 漏洞二：GD 像素爆炸 DoS

• 类型：安全（DoS）
• 风险：🔴 高
• 位置：Upload::makeThumbnailGd()

问题：黑客上传体积极小但分辨率极大的"图片炸弹"（如 10000×10000，压缩后几KB）。imagecreatefromjpeg 为还原位图申请 ~400MB 内存，并发多个请求即可撑爆 PHP memory_limit 或服务器 OOM。

修复：getimagesize 后拦截超大像素：

if ($info[0] > 5000 || $info[1] > 5000) {
    @unlink($sourcePath);
    return;
}

🟠 问题三：MIME 白名单缺失（WebP 绕过风险）

• 类型：安全
• 风险：🟠 中
• 位置：Upload::file()

问题：$allowedMimes 中未定义的扩展名（如 webp 或未来加的 svg）会跳过 MIME 检查。黑客可上传 .webp 后缀的 PHP 木马，MIME 检查被跳过。

修复：补全 webp MIME 映射，并将逻辑改为"无映射表则拒绝"而非"跳过"：

if (!isset($allowedMimes[$ext]) || !\in_array($mime, $allowedMimes[$ext], true)) {
    return ['success' => false, 'error' => '文件类型不匹配'];
}

🟡 问题四：uniqid 文件名可预测

• 类型：安全（隐私）
• 风险：🟡 低
• 位置：Upload::file()

问题：uniqid('', true) 基于微秒时间，非密码学安全随机数。黑客可能推算文件名。

修复：

$randomToken = \bin2hex(\random_bytes(16));
$filename = $randomToken . '_' . \time() . '.' . $ext;

七、修复优先级总表（完整版）