tamakyi/tamabox

Fork 0

Files

T

tamakyi 2ed27dd55e 0904

2026-09-08 10:10:44 +08:00

13 KiB

Raw Blame History

代码审阅报告 —— 9-01 深夜加固批次（凌晨 02:11–02:23）

审阅范围：internal/shared/ip.go、internal/route/ratelimit.go、internal/route/autoban.go、internal/template/markdown.go、internal/form/form.go，及 3 个测试（ip_test.go、markdown_test.go、csrf_binding_test.go）
审阅方式：逐文件源码走查 + 交叉核对调用方契约（route.go 挂载点、login.go uid 键、ipblacklist 精确匹配、模板 FuncMap）+ 第三方库源码级语义核验（[email protected] run()、flamego/[email protected] Validate、net/http 表单惰性解析）
审阅日期：2026-09-02
结论：单个文件实现均正确、自洽，且有测试覆盖；但 form.go 的「请求体 12MiB 上限」因中间件执行顺序在真实链路上失效，属上一轮安全评审 P1-3「磁盘耗尽」未真正闭环；另有一处反序列化失败不中断业务链的通用缺陷。建议修复后再上线（本次仅审阅，未改码）。

一、🔴 P1-1　请求体 12MiB 上限实际失效：超大 multipart 仍可无节流落盘（磁盘耗尽 DoS 未闭环）

位置：internal/form/form.go:59-79（上限安装点）、internal/context/context.go:180-182（CSRF 校验点）、internal/route/route.go:372-388（中间件注册顺序）

问题

maxRequestBodySize 的 http.MaxBytesReader 被安装在 form.Bind 内（路由级 handler），但真实链路上 CSRF 校验（Contexter → x.Validate(ctx)）是 组级中间件、先于一切路由级 handler 执行：

安全头 → IP黑名单 → autoban → cache/recaptcha/session → csrf.Csrfer → templater
      → Contexter（★ 此处对每个 POST 执行 csrf.Validate）
      → form.Bind（★ 12MiB 上限到这一步才装上）→ rateLimit → 业务动作

而 flamego/csrf 的 Validate 取值顺序是先读 Header，无 Header 时 FormValue("_csrf")——后者会触发 net/http 的整包惰性解析：

multipart：ParseMultipartForm(默认 32MiB 内存)，超出部分无上限落盘临时文件；
解析发生在 token 校验之前：即使不带/带错 CSRF token（随后被 400 拒绝），超大 body 也已被完整读入并落盘；
该 400 由 Contexter 直接写出 → 响应已写、链路中断 → 路由级的 rateLimitMiddleware / autoban 计数根本没机会执行。

即：对任意 POST 路由（公开提问框 /_/{domain}、/vote/{token}/submit、/login 等），攻击者无需有效 CSRF token、不受限流约束，持续 POST >32MiB 的 multipart body 即可无限写临时目录，1C1G 机器可在数分钟内被打满磁盘。form.go 注释中想防御的场景（匿名可达接口的磁盘耗尽）依然存在。

补充：urlencoded 大 body 同理在 CSRF 阶段整体载入内存（无上限、无限流），为内存压力向量。

修复建议（单点修复，可同时覆盖下文 P2-2）

在全局 f.Use 层（route.go:146 autobanMiddleware 之后即可）安装统一请求体上限，早于一切组级中间件与 body 解析：

// internal/route/（建议独立文件 bodylimit.go）
const maxRequestBodySize = 12 << 20 // 与 form.go 语义一致；如需多图余量可统一上调

func requestBodyLimitMiddleware() flamego.Handler {
    return func(ctx flamego.Context) {
        req := ctx.Request().Request
        req.Body = http.MaxBytesReader(ctx.ResponseWriter(), req.Body, maxRequestBodySize)
    }
}
// route.go
f.Use(autobanMiddleware(ab))
f.Use(requestBodyLimitMiddleware()) // 必须在 Contexter 之前

取值论证：合法单请求最大 body = 单张图片 ≤8MiB（storage.MaxUploadBytes，internal/storage/s3.go:36，且提问/回答只取 f.Images[0]）+ 表单字段/边界开销，远小于 12MiB；头像/背景 2MiB、私聊图 5MiB 均安全。12MiB 不会误伤合法提交。
该中间件同时封住：form.Bind 路由（含 HTML 表单路径）、chat /send、vote /submit，以及 CSRF 自身触发的解析。form.go 内层 wrap 可保留（幂等，误差同源）或删除。
回归测试建议：仿 route/csrf_binding_test.go 增加路由级测试：向任一 POST 路由发送 Content-Length 超过上限的 multipart body，断言返回 4xx 且不落入业务（可借 f.Post("/protected", ...) 桩验证 body 未被读）。

二、🟠 P2-1　form.Bind 反序列化失败不阻断业务链：零值结构体进入业务动作

位置：internal/form/form.go:70-78（失败分支仅 c.Map(Error{...}) 后 return）

问题

[email protected] run()（context.go:190-228）的链语义是：handler 返回后只要 ResponseWriter().Written() == false，就继续执行下一 handler（没有「不调用 Next 即停链」的语义）。因此 Bind 解析失败分支只 Map(Error)、不 SetError 也不写响应 → 后续业务动作以零值结构体照常执行，ctx.HasError() 恒为 false，govalid 校验被整体跳过。

触发面：body 在 Bind 阶段解析失败（>12MiB、畸形编码）。HTML 表单提交的 body 在 CSRF 阶段已解析成功（Bind 幂等重解析不会失败），实际只影响 Header 携带 CSRF token 的 JS/fetch 提交（token 在头里时 CSRF 不解析 body，解析首次发生在 Bind）。匿名路由攻击者可自持「uid=0」的匿名 token 构造该请求。

各动作影响（按危害排序）：

动作	零值后果	二次防线
`user.UpdateProfile`（`route/user/profile.go:66`）	`db.Users.Update` 用 map 全字段无条件写（`internal/db/users.go:336-357`）→ 昵称/简介/配色/社交链接被清空、`Notify` 置 `None`（静默退订邮件通知）；无任何错误提示	无验证码；仅限账户本人/持其 CSRF 的请求（自伤型数据丢失）
`user.UpdatePagePassword` / `UpdateAskPassword`（`route/user/password.go:17,32`）	这两个 handler 连 `HasError` 都不查，直接 `f.Password`（空=清除密码）→ 反序列化失败等于「关闭页面/提问密码」，防护被静默解除	仅本人（自伤型）
匿名 `question.New` / `PublishFollowUp`（`route/question/page.go:315`）	零值内容绕过 govalid 与屏蔽词/长度校验进入业务；`New` 有验证码闸门兜底，`PublishFollowUp` 无验证码且该路由无限流	验证码开启时安全；站点关闭/未配验证码时可产生空内容垃圾数据
`auth.Register/Login/Forgot/Recover`	零值走 DB 校验失败路径	验证码 + DB 唯一性

修复建议（与既有 govalid 分支同构，改动最小）

internal/form/form.go:71,76 两处失败分支，在 c.Map(...) 前补 c.SetError(err)（或本地化包装，如「请求内容不合法，请重试」）。SetError 会置 HasError=true 并写入 Data["Error"]（internal/context/context.go:71-86），所有动作的 if ctx.HasError() 分支随即生效（重渲染当前页并提示），与校验失败体验一致。

if err := r.ParseForm(); err != nil {
    c.SetError(err)
    c.Map(Error{Category: ErrorCategoryDeserialization, Error: err})
    return
}

注意：UpdatePagePassword/UpdateAskPassword 本身不查 HasError（空密码=关防护是合法操作），若采用 SetError 方案需给这两个 handler 补一个 if ctx.HasError() { ctx.Redirect(...); return } 守卫，否则 SetError 后仍会往下写空密码。

回归测试建议

给 internal/form（或路由级）补用例：模拟 >12MiB 的 multipart body 经 Bind 后，断言 HasError()==true；若约束链上动作，断言动作未执行。

三、🟠 P2-2　chat `/send` 与 vote `/submit` 无任何请求体上限

位置：internal/route/chat/chat_handler.go:127-141、route/vote/vote.go:678+；两路由均不走 form.Bind（route.go:245,257 直接挂 handler）。

问题：SendMessage 用 FormValue("content")/FormFile("image")、Submit 用多个 FormValue(...) 惰性触发解析——在 Header token 路径下（CSRF 不预解析），首次解析发生在 handler 内部、无 MaxBytesReader：multipart 超 32MiB 落盘无上限。私聊图 5MiB 业务上限在解析完成后才检查（chat_handler.go:233）。

缓解因素：chat /send 前置 reqUserSignIn（route.go:248）+ 20/min 限流；vote /submit 前置三层限流。故列为 P2。采用 P1-1 的全局 body 上限方案后本项自动闭环，无需单独改动。

四、✅ 确认无问题项（本批次核心实现 + 测试）

文件	判定	依据
`internal/shared/ip.go`	✅	信任模型正确：`TrustProxy=off` 时仅取 `RemoteAddr`；on 时 CF-Connecting-IP → 可信代理 XFF 剥取。`LimitKey` IPv6 聚合 /64 防轮换绕过；注释明确「不得用于黑名单」——与 `ipblacklist.go` 的 exact/CIDR 匹配一致
`internal/route/ratelimit.go`	✅	滑动窗口 + 全局注册表 `sync.Once` 单清理协程（修复旧版每实例泄漏）；visitors>10000 快速清理守卫；匿名/登录 3 倍分级独立计数；vote 三层限流键无冲突。登录判定 `s.Get("uid").(uint)` 与 login.go 写入类型一致
`internal/route/autoban.go`	✅	`LimitKey` 键与 `f.NotFound` 记录（`route.go:395`）一致，封禁可命中；403 直断（写响应即停链）；敏感路径 `defer Record` 于响应后计数；404 独立计数，避免业务 302→/ 误判
`internal/template/markdown.go`	✅	白名单消毒 + `safeURL`（拒 `//`、`\`、`javascript:` 等）+ `safeColorValue`；`MarkdownToHTML`/`PlainText` 与模板 FuncMap 注册一致；测试覆盖 XSS/协议相对 URL/字体色注入
`csrf_binding_test.go`	✅	守卫 `CSRFUserSessionKey="uid"` 与 login 绑定一致（误改回 "userID" 即失败）；会话 B 复用 A 的 token → 400
`ip_test.go` / `markdown_test.go`	✅	覆盖充分（IPv4 透传、v4-mapped 折叠、/64 聚合、端口、非法/空；markdown 六类用例）

次要观察（不改）：rateLimiter.allow() 的 >10000 清理仅删「窗口内已无活动」键，活动大流量下 map 仍可增长——受 LimitKey 聚合 + 短窗口约束，实际可达量级（数万键 ≈ 数 MB）可接受，非回归。

五、✅ 修复实施记录（2026-09-02 夜，已落地并验证）

上节为原评审建议（针对 02:11–02:23 深夜加固批次）。用户批准后已全部实施，P2-2 随 P1-1 自动闭环。复核中发现原 P2-1 影响面比初判更大：UpdateHarassment / UpdateBlockWords / UpdateTags 与两个 password handler 一样不检查 HasError，零值结构体会静默关闭防骚扰 / 清空屏蔽词 / 清空标签——一并纳入守卫。

编号	改动文件	内容	验证
P1-1	`internal/route/bodylimit.go`（新增）	全局 `requestBodyLimitMiddleware`：POST/PUT/PATCH 且 `ContentLength>12MiB` → 413 快速失败；其余用 `http.MaxBytesReader` 惰性封顶。挂载于 `f.Use` 链（autoban 之后、所有 Group 之前），先于 Contexter 内 CSRF 的整包惰性解析	`go build -overlay` ✅
P1-1	`internal/route/route.go`	`f.Use(requestBodyLimitMiddleware())`（autoban 之后）	全模块编译 ✅
P1-1	`internal/form/form.go`	`maxRequestBodySize` 导出为 `MaxRequestBodySize`（单一事实来源，供全局中间件引用）	`go vet` ✅
P1-1	`internal/route/bodylimit_test.go`（新增）	回归测试：合法 8MiB → 200；Content-Length 超限 → 413；chunked 超限（绕过预检）→ MaxBytesReader 拒绝	`go test ./internal/route/` ✅
P2-1	`internal/form/form.go`	两个反序列化失败分支（ParseForm / ParseMultipartForm）补 `c.SetError(err)`——置位 HasError，使检查 HasError 的 action（含匿名提问/回答/追问）在零值结构体进入业务前早退	同上
P2-1	`route/user/harassment.go`	新增 `abortOnFormError` helper（HasError → 错误经 flash 转交 → redirect 中止）；`UpdateHarassment`、`UpdateBlockWords` 开头加守卫	同上
P2-1	`route/user/tags.go`	`UpdateTags` 开头加守卫（防清空标签 + 关闭首页展示）	同上
P2-1	`route/user/password.go`	`UpdatePagePassword` / `UpdateAskPassword` 开头加守卫（防空密码静默解除页面/提问防护；govalid 超长密码此前也会绕过前端限制直接入库，一并拦截）	同上
P2-2	—	chat `/send`、vote `/submit` 已被全局中间件覆盖（超限时 CSRF/解析失败即拒），无需单独改动	见 P1-1

测试结果（go test -overlay=dev/overlay.json）：templates、internal/template、internal/form、internal/shared、internal/security/autoban、internal/security/censor、internal/route（含新增 bodylimit 回归 + csrf_binding）全部 PASS；go build ./... 与 go vet ./internal/... ./route/... exit 0。

附：本报告评审结论基于源码走查与依赖库源码语义核验；修复部分已于 2026-09-02 夜落地并通过 overlay 全模块编译 / vet / 相关包测试。

13 KiB Raw Blame History Unescape Escape