tamakyi
e3e59e7c69
Repo .git was damaged (refs/ missing, index truncated, object store near-empty). Repaired via git init (rebuilt refs), stale index preserved as .git/index.corrupt-20260910.bak, then full worktree committed as a rollback baseline.
114 lines
11 KiB
Markdown
114 lines
11 KiB
Markdown
# TamaBox-v4 完整代码审阅报告
|
||
|
||
> 审阅时间：2026-08-27
|
||
> 审阅对象：`TamaBox-v4-new-toupiao`（基于 NekoBox 的 Go 匿名问答 + 匿名投票 Web 应用，Flamego + GORM + MySQL/PostgreSQL + Redis）
|
||
> 审阅范围：全项目 Go 源码（约 20.8k 行，86 个 internal 文件 + 21 个 route 文件 + cmd）、关键模板、路由/中间件、配置
|
||
> 审阅方式：针对性精读（认证/会话、投票、私聊、提问、个人资料、Markdown 清洗、缓存、路由/中间件、限流）+ 全代码库自动化广度扫描 + 逐项复检历史报告（08-13 / 08-18 / 08-24 / 08-26）+ 核对 Flamego 框架源码确认中间件语义
|
||
> 结论口径：仅就当前代码可直接确认的问题定性，未做全量编译/运行（本环境缺 CGO/libwebp 与测试数据库，与 08-26 报告一致）
|
||
|
||
---
|
||
|
||
## 一、总体结论
|
||
|
||
**当前代码质量高、安全水位扎实，历史报告中的 P0/P1 缺陷已全部修复，未发现可阻断上线的 P0/P1 漏洞。** 剩余问题集中在 **健壮性 / 可扩展性（P2）** 与 **防御纵深缺口 / 易用性（P3）**，无直接可利用的高危漏洞。
|
||
|
||
本项目在安全意识上明显优于一般同类项目：CSRF 全覆盖、会话密码版本失效、图片像素/解压炸弹防护、代理 SSRF 三处防护、内容审核 fail-closed、匿名投票复合唯一索引 + HMAC 签名指纹、SafeGo 防崩溃、私聊原子计数 + 回滚、CSV 公式注入防护等均已落地。
|
||
|
||
---
|
||
|
||
## 二、历史问题复检（相对 08-13 / 08-18 / 08-24 / 08-26）
|
||
|
||
逐条核对后，**此前报告中列为 P0/P1/P2 的问题在当前代码中均已处理**：
|
||
|
||
| 历史问题 | 状态 | 当前证据 |
|
||
|---|---|---|
|
||
| 真实云服务/数据库密钥入库 | 已知未处理 | 按用户要求保留（测试环境凭据），非本次新增 |
|
||
| 图片解压炸弹（OOM） | ✅ 已修复 | `internal/imagecheck` 像素校验 + 三处解码入口接入 |
|
||
| 6 位 token 可爆破 | ✅ 已缓解 | 投票 `LinkToken` 用 `randstr.String(12)`；私聊 `Conversation.Token` 6 位（旧约定，低风险） |
|
||
| PostgreSQL schema 硬编码 | ✅ 已修复 | `fixPostgresSequences` 改用 `conf.Database.Schema` |
|
||
| 密码修改后旧会话仍有效 | ✅ 已修复 | `PasswordVersion` + `authenticatedUser` 版本校验 |
|
||
| 找回密码邮箱枚举 | ✅ 已修复 | 邮箱不存在也渲染统一「已发送」页 |
|
||
| 内容审核 fail-open（08-24 P1-1 两处） | ✅ 已修复 | `question.go:136-149`、`page.go:317-329`、`page.go:444-458`、`route/vote/censorCheck` 均在 `err != nil` 时拒绝发布 |
|
||
| 匿名 `vote_fp` Cookie 可伪造（08-26 P1-01） | ✅ 已修复 | `route/vote/vote.go:83-93` `validVoterToken` 做 HMAC 签名校验；`newVoterToken` 签名 |
|
||
| Markdown 实体编码 XSS（08-26 P1-02） | ✅ 已修复 | `internal/template/markdown.go:147` `sanitizeHTML` 对 `TextToken` 调用 `html.EscapeString`；`markdown_test.go:35` 含 XSS 回归测试 |
|
||
| 多选重复选项重复计票（08-26 P2-01） | ✅ 已修复 | `parseMultipleOptionIDs`（`vote.go:753-780`）用 `seen` 集合去重 |
|
||
| `max_votes_per_fingerprint` 配置矛盾（08-26 P2-02） | 🟡 已缓解 | 业务规则固定为一人一票（`parseVoteMeta` 硬编码 `maxVotes=1`，UI 无控制）；但 DB 列与模板默认值仍残留（见 P3-5） |
|
||
|
||
---
|
||
|
||
## 三、新发现的问题（本次审阅）
|
||
|
||
### P2（中危 / 健壮性·可扩展性）
|
||
|
||
**P2-1 投票结果/CSV 全量加载所有提交记录（O(N) 内存+CPU）**
|
||
`LoadResults`（`route/vote/vote.go:209`）对 multiple/rank/rating 类型，以及 `aggregateFormFieldStats` / `aggregateCustomStats` / `VoteResults` / `VoteExportCSV` 均使用 `db.VoteSubmissions.ListByVoteID(ctx, voteID, nil)`（无游标、无上限）把**全部提交记录载入内存**再逐条 `json.Unmarshal` 聚合。结果虽有 30s 进程内 TTL 缓存，但每个缓存窗口的首次计算对热门投票（数千~数万条）是一次性内存/CPU 尖峰。配合 P2-2 的灌票风险，可能放大为资源耗尽。
|
||
**建议**：聚合逻辑下推 SQL（`COUNT ... GROUP BY option_id` 已有；rank/rating/form 应补 `SUM/AVG` 或分页流式聚合）；或限定单次加载行数上限并降级展示。
|
||
|
||
**P2-2 匿名投票防刷阈值偏高 + 清 Cookie 可绕过 token 维度限流**
|
||
`voteSubmitRateLimitMiddleware`（`internal/route/ratelimit.go:151`）对匿名用户：
|
||
- IP 维度兜底 = `limit*10` = **200 次/分钟**；
|
||
- token 维度 = `limit` = 20 次/分钟，但**仅当请求携带有效签名 `vote_fp` 时才计数**。
|
||
|
||
若箱主关闭验证码（`RequireCaptcha=false`，DB 默认 `true` 但可由表单关闭），攻击者在每次提交时发送无效/清空 `vote_fp` Cookie，`ensureVoterToken` 会签发一个**全新的有效签名 token**，token 维度限流永不触发，仅 IP 维度 200/min 兜底生效 → 单 IP 每分钟可灌入最多 200 张真实票，污染结果统计。
|
||
**建议**：匿名投票始终要求验证码（或行为验证码）；降低 IP 兜底阈值；或改为服务端持久化指纹（绑定 IP+UA 哈希）替代纯客户端 Cookie。
|
||
|
||
### P3（低危 / 防御纵深·易用性）
|
||
|
||
**P3-1 autoban 的 404 探测自动封禁失效**（经 Flamego 源码核实）
|
||
`internal/route/autoban.go:46-58` 在 `defer` 中读取 `rw.Status()` 判断 404 探测并 `b.Record(KindNotFound)`。但 Flamego 的调度（`[email protected]/context.go:190-229`）在**每个 handler 返回后、下游执行前**就运行 `defer`，且此时 `Written()==false`、`Status()==0`，因此 `KindNotFound` 分支**永远不触发**——404 探测型自动封禁形同虚设。敏感路径封禁（`KindSensitive`，在 defer 前同步设置）仍正常。
|
||
**建议**：在 allow 分支显式调用 `ctx.Next()`，使 `defer` 在下游 handler 执行后才运行；或在 `NotFound` 钩子里直接统计 404 信号。
|
||
|
||
**P3-2 进程内结果缓存跨实例不一致 + 新提交延迟最多 30s**
|
||
`resultsCache` / `voteTextRepliesCache` 为单进程 TTL 缓存（`internal/cache/cache.go` 已确认 `sync.RWMutex` 并发安全）。多实例横向扩容时各实例独立缓存，结果会发散；且提交成功后未主动失效，参与者「投票后查看结果」最多延迟 30s 才反映自己的票。
|
||
**建议**：多实例改用 Redis 共享缓存（项目已用 flamego/cache-redis）；或在 `VoteSubmissions.Create` 成功后删除对应 `vote.ID` 的结果缓存 key。
|
||
|
||
**P3-3 死配置字段 `max_votes_per_fingerprint`**
|
||
业务逻辑已固定为一人一票（`parseVoteMeta` 硬编码 `maxVotes=1`，UI 无控制项），但 `db.Votes.MaxVotesPerFingerprint` 列仍存在并被写入，`VoteNew` 仍设默认值 1。无功能影响，但易误导维护者以为可配置多票。
|
||
**建议**：移除该列与模板默认值，或补 UI/文档明确「一人一票」。
|
||
|
||
**P3-4 草稿/归档投票 POST `/vote/{token}/submit` 提示误导**
|
||
`route/vote/vote.go:647-655`：`isVoteActive` 为 false 且非 ended 时统一提示「该投票尚未开始」。对 `draft`/`archived` 状态的投票（其链接本不该被提交），该提示不准确（实际应提示「不存在或已失效」）。
|
||
**建议**：按 `vote.Status` 区分提示文案。
|
||
|
||
**P3-5（信息项，非缺陷）已确认无误的点**
|
||
- 复合唯一索引 `(vote_id,user_id)` / `(vote_id,voter_token)` 利用 MySQL/PG 的 NULL 不冲突语义正确实现一人一票；`VoteSubmissions.Create` 用 `isDuplicateKeyError` 将唯一冲突转为 `ErrVoteAlreadyVoted` 兜底 TOCTOU。
|
||
- `db.User` 的 `Password`/`PasswordVersion`/`PagePassword`/`AskPassword` 均为 `json:"-"`，`ProfileAPI`（`ctx.JSON(ctx.User)`）与 `ExportProfileJSON` 均**不泄漏口令哈希**。
|
||
- `resultsCache` 返回共享 `*Results` 指针，构造后未被变异，当前安全；属脆弱点（未来若在某 handler 内修改该对象会影响所有读者）。
|
||
|
||
---
|
||
|
||
## 四、改进方向（非缺陷，建议项）
|
||
|
||
1. **聚合下推 SQL**：投票结果统计（rank/rating/form）改为数据库层聚合或分页流式处理，消除热门投票的内存/CPU 尖峰（对应 P2-1）。
|
||
2. **匿名指纹防刷增强**：持久化指纹 + 始终验证码 + 更严格的 IP 阈值，替代纯客户端 Cookie（对应 P2-2）。
|
||
3. **缓存失效与一致性**：提交成功后主动失效结果缓存；多实例用 Redis 共享（对应 P3-2）。
|
||
4. **自动化测试补齐**：增加集成/E2E 用例覆盖——内容审核 fail-closed、并发一人一票（唯一索引兜底）、`vote_fp` 签名防伪造、SSRF 逐跳校验、CSV 公式注入、autoban 敏感路径封禁。
|
||
5. **CI 质量门禁**：引入 `golangci-lint` + `govulncheck`；当前 `go test ./...` 因 CGO/libwebp 与未配置测试 DB 无法全绿，建议在带 C 工具链与测试库的预发布环境跑全量 `go test ./...`、`go vet ./...`、`govulncheck ./...`。
|
||
6. **清理与一致性**：移除死配置 `max_votes_per_fingerprint`、修正草稿/归档投票提交提示、统一错误日志与监控指标（若干 `_ =`/忽略错误可改为显式 metric）。
|
||
|
||
---
|
||
|
||
## 五、审阅覆盖清单
|
||
|
||
| 模块 | 审阅结论 |
|
||
|---|---|
|
||
| 认证/会话（`route/auth`、`internal/context/auth`、`internal/db/users`） | 密码版本失效、会话重生成到位；口令哈希 `json:"-"` 不泄漏 |
|
||
| 投票参与端（`route/vote/vote.go`） | 一人一票（唯一索引+签名 token）、fail-closed 审核、CSRF、去重、限流到位；P2-1/P2-2 待优化 |
|
||
| 投票管理端（`route/user/vote.go`） | `getOwnedVote` 全链路归属校验；草稿/发布分级正确；P3-4 提示待修 |
|
||
| 私聊（`internal/route/chat`） | 原子计数+回滚、归属校验、fail-closed 审核、邮件节流，均正确 |
|
||
| 提问（`route/question`） | 审核 fail-closed、token 访问控制、追问等待逻辑正确 |
|
||
| Markdown 清洗（`internal/template/markdown.go`） | 实体编码 XSS 已修复（EscapeString） |
|
||
| 缓存（`internal/cache`） | 并发安全（RWMutex）；P3-2 跨实例/失效待优化 |
|
||
| 路由/中间件/限流（`internal/route`） | 限流/IP 封禁/自动封禁正确拦截（WriteHeader 即停链）；P3-1 autoban 404 探测失效 |
|
||
| 个人资料/导出/停用（`route/user/profile.go`） | 仅操作 `ctx.User.ID`，无 IDOR；导出不泄漏口令 |
|
||
| 模板 XSS 输出 | `QuestionFormat`/`AnswerFormat` 已 EscapeString；仅 CSRF token 用 `Safe`（服务端生成）|
|
||
|
||
---
|
||
|
||
## 六、优先级建议
|
||
|
||
1. （可选，按需）P2-1 / P2-2：若投票功能面向公网且预计有热门投票，优先加固匿名防刷与结果聚合性能。
|
||
2. P3-1：修正 autoban 404 探测（一行 `ctx.Next()` 改动，恢复防御纵深）。
|
||
3. P3-2 ~ P3-4：随迭代清理，不阻塞上线。
|
||
4. 上线前在完整 CI 环境跑全量测试与漏洞扫描，确认无回归。
|