tamakyi/tamabox
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
6ffc87e4a8b7ec3343b9409934f6c7952f842826
tamabox/outputs/code-review-2026-08-31.md
T
tamakyi e3e59e7c69 chore: baseline snapshot before moderation-strictness feature 
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.
2026-09-10 10:56:27 +08:00

133 lines
13 KiB
Markdown
Raw Blame History

This file contains invisible Unicode characters
This file contains invisible Unicode characters that are indistinguishable to humans but may be processed differently by a computer. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# TamaBox-v4 整体功能与逻辑审阅报告
- **审阅日期**：2026-08-31
- **范围**：全仓库（route / internal / templates / conf）
- **方法**：静态走查 + 编译验证（`go build` / `go vet` / 单元测试）+ 对每项高危结论做源码核对（trust but verify）
- **结论概览**：核心架构扎实（路由/中间件/会话固定/回收令牌/配额原子递增/功能开关 Evaluate 逻辑均正确）。发现 **1 个 P0 级安全缺陷（IP 伪造绕过）、若干 P1 逻辑/安全/可用性问题、一批 P2 健壮性项**。无导致编译崩溃的致命代码错误。
---
## 0. 测试与构建验证结果
| 项目 | 结果 |
|---|---|
| `go version` | go1.25.5 windows/amd64 |
| `go build ./...` | ❌ 仅因 `internal/storage/s3.go` 依赖 `github.com/chai2010/webp`（CGO 缺 libwebp 本机库，沙箱限制，与既有约定一致）。**其余包均可独立编译** |
| 核心包编译 | ✅ `internal/form internal/db internal/cache internal/i18n internal/captcha internal/security/* internal/context internal/conf internal/dbutil` 全部 OK |
| `go vet`（上述包） | ✅ 无告警 |
| 单元测试 | ✅ `internal/form` `internal/i18n` `internal/cache` `internal/security/autoban` 全部通过 |
| DB 集成测试 | ⚠️ 需外部 PostgreSQL/MySQL（`DB_TYPE` 环境变量，沙箱无），未执行；`feature_control_test` 等需落库测试建议在有 DB 环境补跑 |
> 说明：`gofmt -l` 发现 11 个文件未格式化（如 `internal/security/censor/text.go`、`internal/captcha/math.go`、`route/user/harassment.go` 等），建议 `gofmt -w` 统一。
---
## 🔴 P0 — 安全/可用阻断级
### P0-1　反向代理 IP 信任可被伪造，限流/黑名单/自动封禁全线失效
- **位置**：`internal/shared/ip.go:42-69`（`ResolveClientIP`）、被 `internal/route/ratelimit.go`、`autoban.go`、`ipblacklist.go` 共用
- **问题**：当 `conf.Security.TrustProxy=true` 时，直接信任客户端发来的 `CF-Connecting-IP` / `X-Real-IP` / `X-Forwarded-For`，**不校验 `RemoteAddr` 是否属于可信代理 CIDR**。任何人都可伪造这些头。
- **影响**：
1. 限速、IP 黑名单被完全绕过（每次请求换一个伪造 IP）；
2. 自动封禁（fail2ban）被反射利用——攻击者用伪造头把**任意受害者 IP** 打进黑名单，造成误封（含 CDN 出口 IP，可能误伤全站访客）。
- **现实性**：`conf/app.sample.ini` 明确建议"站点在 Cloudflare 后且需自动封禁时必须设 `trust_proxy = true`"——这正是绝大多数生产部署，故该缺陷在生产可达。
- **修复**：配置 `trusted_proxies` CIDR 列表；仅当 `RemoteAddr` 命中可信网段才读取代理头；`X-Forwarded-For` 自右向左剥离已确认的可信跳数后再取最左原始 IP。
---
## 🟠 P1 — 逻辑错误 / 漏洞 / 可用性
### P1-1　会话 Cookie 缺 `Secure` / `SameSite`
- **位置**：`internal/route/route.go:92-95`（`session.Options` 仅设 `Initer/Config`，无 `Cookie` 字段）
- **影响**：生产经 TLS 反代时，会话 Cookie 不会标 `Secure`（明文链路可泄露）；未显式 `SameSite`。`conf.App.Production` 只切 Flamego 环境，不联动 Cookie 标志。
- **修复**：按 `conf.App.ExternalURL` 是否 `https` 设 `Secure:true`；`SameSite:http.SameSiteLaxMode`（与既有 CSRF Token 协同）；`HttpOnly:true`。
### P1-2　配置解析：行内 `;` 注释污染值，最坏导致启动崩溃
- **位置**：`internal/conf/conf.go:25-27`（`IgnoreInlineComment:true`）+ `conf/app.sample.ini:5,11,12,16-28,54`
- **问题**：该选项下 `key = val ; 注释` 的注释会被并入字段值。示例里 `trust_proxy = false ; 是否信任…`、`ban_threshold_sensitive = 10 ; …` 等带行内注释。若把 sample 直接当 `app.ini` 用，`File.Section("security").MapTo(&Security)` 解析 bool/int 会因尾部中文注释失败 → **进程启动即崩溃**；`share_background` 这类字符串字段则会得到带注释的非法 URL。
- **修复**：将 `IgnoreInlineComment` 改为 `false`（安全默认，`;` 重新开始注释），或全量清除 sample 中的行内注释（当前 sample 与其自身约定"禁用行内注释"矛盾）。
### P1-3　本地算数验证码可被脚本自动解出 + 端点无节流
- **位置**：`internal/captcha/math.go:195-212`（`GenerateHandler` 无限流）、`:148-159`（`VerifyOnly` 不消费、不计数）、`:49-55`（忽略 `rand.Int` 错误）
- **问题**：验证码题目由服务端返回给客户端（如 `"23 × 7 ="`），答案对客户端完全可见，**任何脚本都能读题即算**——对机器人无实质阻挡；`/api/captcha/verify` 无节流且 `VerifyOnly` 不消费，可被无限轮询；`GenerateHandler` 无限流会导致会话条目无界膨胀（DoS）。
- **影响**：匿名投票/提问的"唯一门槛"被自动化绕过；叠加 **P0-1** 的 IP 伪造，匿名刷票/刷提问几乎无成本。
- **修复**：匿名提交改用语服务端校验的 Turnstile/recaptcha；或补 `/api/captcha/*` 限流 + 同 id 尝试 3 次即销毁；对 math 答案做常量时间比较。
### P1-4　追问"等主人回复"仅客户端校验，服务端可绕过
- **位置**：`route/question/follow_up.go:31-63`（`PublishFollowUp`）
- **问题**：仅检查 `question.Answer != ""` 与 `count < 3`，**不校验上一条追问是否已被回答**（`waitingFollowUpReply` 只在模板隐藏表单，POST 不强制）。攻击者可连发满 3 条追问而不等主人回任一条。
- **修复**：取该问题最新一条 follow-up，若 `Answer == ""` 直接拒绝（并复用 `CreateIfBelowLimit` 原子兜底）。
### P1-5　内容审核"可信域名剥离"被绕过，钓鱼链接漏审
- **位置**：`internal/security/censor/text.go:89-96`（`RemoveTrustedURL`）
- **问题**：朴素 `strings.ReplaceAll("https://github.com", "")`。`https://github.com.evil.tld/phish` 会被剥成 `.evil.tld/phish`，恶意域名逃过 URL 审核。
- **修复**：解析 URL 后按"精确 host 或受信 host 的子域"精确匹配，再决定是否跳过审核。
### P1-6　密码校验：遗留 HMAC 比较非常量时间 + bcrypt 失败静默降级
- **位置**：`internal/db/password.go:56`（`legacyHash == storedHash`）、`:24`（`GenerateFromPassword` 出错回退弱 HMAC-SHA1）
- **问题**：遗留账户走 `==` 比较存在时序侧信道；bcrypt 出错时不返回错误，反而存一个弱 HMAC-SHA1 哈希（下次登录按 legacy 处理）， silently 削弱了密码存储强度。
- **修复**：遗留分支改用 `subtle.ConstantTimeCompare`；bcrypt 失败直接 `return err`，不回退。建议 bcrypt cost 提到 12。
### P1-7　删除内容不回退功能配额计数
- **位置**：`internal/db/questions.go`（删除）、`internal/db/conversation.go:204`（`MarkDeletedByOwner`）
- **问题**：达定量上限后，即便删除该提问/私聊，对应 `feature_controls.quota_count` 不 `-1`，功能仍显示 `quota_reached` 关闭，须箱主手动"重置计数"。
- **修复**：删除时按功能类型调用 `FeatureControls.IncrementQuota(-1)`（注意软删除与并发）。
### P1-8　自动封禁把合法 302 误判为探测
- **位置**：`internal/route/autoban.go:51-53`
- **问题**：把"302 → /"一律当扫描信号；但 `/i18n/{lang}` 切换、logout、黑名单跳转、投票不存在等都是 302→/。60s 内 20 次即 403 封 15 分钟，易误伤正常用户。
- **修复**：仅在 `f.NotFound`（真正的 404）路径内计数，排除业务性 302。
### P1-9　服务反代路径穿越 + 丢失 query + 无超时
- **位置**：`route/service/proxy.go:37,59`
- **问题**：`forwardPath` 未经 `path.Clean`，`/service/<prefix>/../../x` 可越过白名单前缀（白名单本身有效，无任意 SSRF/元数据风险）；丢失 `RawQuery`；每请求新建 `Transport` 且无超时。
- **修复**：`Clean` 后再次校验仍以 `forwardURL.Path` 为前缀；透传 query；复用带超时的 `Transport`。
### P1-10　`/api/v1/pixel/**` 豁免 CSRF + 放行写方法 + 无节流 + 响应头透传
- **位置**：`internal/context/context.go:170`（CSRF 排除）、`internal/route/route.go:296`（`f.Any` + `reqUserSignIn`）
- **问题**：该前缀跳过 CSRF 校验且 `f.Any` 放行 POST/PUT/DELETE；`path.Join("/api/", uri)` 可经 `..` 跳出 `/api`；上游 `Set-Cookie` 等响应头原样回传；无速率限制。
- **修复**：强制 CSRF 或仅放行 GET；拒绝含 `..` 的路径；白名单放行响应头；补限流。
---
## 🟡 P2 — 健壮性与改进（建议项）
| # | 位置 | 问题 | 建议 |
|---|---|---|---|
| P2-1 | `route/vote/vote.go:310` | 排序题统计 `json.Unmarshal(&data.Ranking,…)` 应为 `&data`，解析必失败且错误被忽略 | 改为 `&data` 并补单测验证 |
| P2-2 | `route/vote/vote.go:48,343,776` | 结果缓存共享指针，且仅新提交失效（删提交/改选项不失效），多实例不一致 | 失效键扩展；或缓存按 vote 维度加版本号 |
| P2-3 | `internal/db/vote_submissions.go:51-55` | 唯一索引不含 `deleted_at`：软删提交后统计归零却无法重投 | 唯一约束纳入 `deleted_at` 或改用业务去重 |
| P2-4 | `internal/db/questions.go` | `Question.Token` 仅 6 位且无唯一索引约束说明，碰撞可越权看他人私密/未答提问 | 确认是否 `uniqueIndex`；不足则加长 |
| P2-5 | `route/question/question.go` | 普通 `Delete` 不级联 `follow_ups`/`UploadImages`，留孤儿记录 | 软删时同步标记子表 |
| P2-6 | `internal/db/users.go` | `CountTodayMessagesFromUser` 用 `CURRENT_DATE`（DB 时区），与 App 本地时区不符，每日私信上限窗口偏移 | 用 App 本地时区日期边界 |
| P2-7 | `route/user/profile.go:337` | 导出 XLSX/CSV 时用户串直写单元格，含 `= + - @` 开头可在 Excel 执行公式（公式注入） | 单元格前加 `\t`/单引号或置文本格式 |
| P2-8 | `internal/db/users.go:454` | 注册重复 email/domain 校验存在 TOCTOU，并发同值双过 `validate` | 包事务或友好去重（唯一索引兜底） |
| P2-9 | `internal/route/gzip.go:75-80` | `ct` 求值后未用 → 未显式设 `Content-Type` 的响应永不压缩；204/304 仍加 `Content-Encoding` | 显式压缩判断 + 跳过无实体状态码 |
| P2-10 | `internal/route/route.go:233` | `/i18n/{lang}` 仅拦 `//`，未拦 `/\` 反斜杠等价变体 | 归一化 `to` 后再校验 |
| P2-11 | `internal/route/route.go` | 缺 HSTS；`lang` cookie 无 `Secure` | 加 `Strict-Transport-Security`；cookie `Secure` |
| P2-12 | `internal/route/chat/chat_handler.go:122` | `SendMessage` 未重新校验 `FeatureChat.Open`（创建时查过，发消息漏查） | 发送前 `FeatureStateOf(FeatureChat).Open` 校验，或明确设计为既有会话豁免 |
| P2-13 | `internal/db/questions.go` | 提问列表 `PinFirst` 与 `id<cursor` 游标冲突，置顶后"加载更多"会漏/重较新非置顶项 | 置顶与分页采用稳定排序键 |
| P2-14 | `internal/captcha/math.go:49` | 忽略 `rand.Int` 返回错误，极端下 `.Int64()` 可能 nil 解引用 | 校验错误并 return |
---
## ✅ 已核对正确的关键点（信心项）
- **私聊消息计数并发**：`IncrementMessageCountIfBelow`（`conversation.go:124`）使用原子条件更新 `WHERE message_count < max AND is_closed=false`，**无超额竞态**（经核实，非 agent 所报的竞态）。
- **提问归属鉴权**：`Questioner` 中间件 + `SetPrivate/Delete/TogglePin/SetRead` 均以 `ctx.User.ID==pageUser.ID` 判定，无 IDOR。
- **投票提交防绕过**：`Submit` 已校验 `FeatureVote.Open`、`isVoteActive`、已投（`HasUserVoted`/`HasTokenVoted`）+ 数据库唯一索引兜底；voter token 为 HMAC 签名。
- **回收密码令牌**：64 位随机、24h 有效期、单次使用、绑定 `user.ID`，设计安全。
- **会话固定**：登录后 `RegenerateID` 已做。
- **配额计数**：`IncrementQuota` 原子 `quota_count + ?` + 并发补加重试，设计合理。
- **启动健壮性**：`AutoMigrate` 前置去重、功能开关 backfill、旧版私聊零值陷阱纠偏，均为幂等且不阻断启动。
---
## 修复优先级建议
1. **立即（P0）**：P0-1 反向代理 IP 信任校验。
2. **本周（P1）**：P1-1 会话 Cookie 标志、P1-2 配置解析、P1-3 验证码策略（配合 P0-1）、P1-4 追问服务端校验、P1-5 审核域名绕过、P1-6 密码比较、P1-8 自动封禁误判、P1-10 pixel CSRF。
3. **排期（P2）**：导出公式注入、统计 bug 单测、缓存一致性、时区等。
> 注：outputs/ 下已有 2026-08-26/08-28 的历史审阅文档，本次为独立复检，新增/重申了 IP 伪造、审核 fail-closed、追问绕过、审核域名绕过、配置崩溃等项；其中 chat 消息计数竞态经核实**不存在**，已在历史结论中纠正。
Reference in New Issue View Git Blame Copy Permalink