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.
254 lines
17 KiB
Markdown
254 lines
17 KiB
Markdown
# TamaBox v4 全量代码安全审阅报告
|
||
|
||
- **审阅日期**：2026-09-02
|
||
- **代码规模**：123 个 Go 文件 / 约 40 个 HTML 模板
|
||
- **技术栈**：Flamego + GORM + PostgreSQL/MySQL + Redis
|
||
- **审阅方式**：静态代码审计（6 个领域并行）+ `go build` / `go vet` / `go test -race` + **真实启动服务做动态渗透验证**
|
||
- **凭据处理**：全程未读取 `conf/app.ini`；服务日志经正则脱敏后才进入分析，配置内容未外传
|
||
|
||
---
|
||
|
||
## 一、总体结论
|
||
|
||
| 项目 | 结果 |
|
||
|---|---|
|
||
| `go build ./...` | ✅ 通过，无错误 |
|
||
| `go vet ./...` | ✅ 完全干净，零告警 |
|
||
| `go test -race ./...` | ✅ **无任何 DATA RACE**（`internal/db` 因缺数据库环境变量失败，非代码缺陷） |
|
||
| 实际运行 | ✅ 成功启动并连接数据库，监听 5877 端口，功能正常 |
|
||
| SQL 注入 | ✅ 未发现 |
|
||
| IDOR / 越权 | ✅ 未发现，归属校验完备 |
|
||
| SSRF | ✅ 防护完备（已实测拦截） |
|
||
| XSS | ✅ 主链路 sanitize 扎实 |
|
||
|
||
**这是一个安全基线明显高于同类项目的代码库**：bcrypt 密码哈希、登录后重建 session ID、CSRF 全局校验、SSRF fail-closed 白名单 + DNS rebinding 防护、开放重定向多变体防护、限流 + 自动封禁、CSV 公式注入转义、敏感字段 `json:"-"`，这些都做对了。
|
||
|
||
**未发现可被匿名攻击者直接利用的 P0 漏洞。** 以下为 5 个 P1 与 12 个 P2 项。
|
||
|
||
---
|
||
|
||
## 二、P1 问题（重要，建议优先修复）
|
||
|
||
### 🔴 P1-1 CSRF token 未与用户会话绑定（已动态验证可跨会话复用）
|
||
|
||
**位置**：`internal/route/route.go:349` 与 `route/auth/login.go:74`
|
||
|
||
```go
|
||
// route.go:349 —— 未指定 SessionKey
|
||
csrf.Csrfer(csrf.Options{
|
||
Secret: conf.Server.XSRFKey,
|
||
Header: "X-CSRF-Token",
|
||
})
|
||
|
||
// login.go:74 —— 实际写入的 key 是 "uid"
|
||
ctx.Session.Set("uid", user.ID)
|
||
```
|
||
|
||
**根因**：`flamego/[email protected]` 的 `defaultSessionKey = "userID"`（`csrf.go:150`）。应用写入的却是 `"uid"`，导致 `s.Get("userID")` 恒为 `nil`，进而 `x.id` 恒为字符串 `"0"`（`csrf.go:198-202`）。而 token 校验是**无状态 HMAC 重算**，并不与 session 中存储的 token 比对：
|
||
|
||
```go
|
||
// csrf.go:62 —— 校验只依赖 secret + c.id("0") + "POST" + 时间戳
|
||
func (c *csrf) ValidToken(t string) bool {
|
||
return ValidToken(t, c.secret, c.id, http.MethodPost)
|
||
}
|
||
```
|
||
|
||
**动态验证结果**（本地实测，非推测）：
|
||
|
||
| 测试用例 | 结果 |
|
||
|---|---|
|
||
| 会话 B 的 cookie + **会话 A 的 token** | `HTTP 200` — **通过校验**，进入业务逻辑 |
|
||
| 会话 B 的 cookie + 伪造 token | `HTTP 400 Bad Request: invalid CSRF token` |
|
||
| 同一个 token 连续复用 26 次 | 全部通过（token 有效期 24 小时） |
|
||
|
||
即：**任意匿名访客从公开的 `/login` 页面拿到的 token，对任何已登录用户都有效**，CSRF 从「每会话密钥」退化为「全站公开常量」。
|
||
|
||
**缓解因素**：session cookie 实测带 `SameSite=Lax`，可阻断经典跨站表单 POST。因此未定为 P0。但一旦出现子域接管、站内 HTML 注入，或将来因兼容性调整为 `SameSite=None`，防线即完全失效。
|
||
|
||
**修复**（一行）：
|
||
```go
|
||
csrf.Csrfer(csrf.Options{
|
||
Secret: conf.Server.XSRFKey,
|
||
Header: "X-CSRF-Token",
|
||
SessionKey: "uid", // 与 login.go 写入的 key 对齐
|
||
})
|
||
```
|
||
修复后匿名 token（id=`"0"`）将无法通过登录用户（id=`uid`）的校验。**建议同时增加一条回归测试**锁定该行为，避免日后 key 改名再次错配。
|
||
|
||
---
|
||
|
||
### 🔴 P1-2 Session cookie 缺少 `Secure` 标志
|
||
|
||
**位置**：`internal/route/route.go:111-114`
|
||
|
||
```go
|
||
sessioner := session.Sessioner(session.Options{
|
||
Initer: initer,
|
||
Config: sessionStorage,
|
||
// 缺少 Cookie 配置，Secure 默认为 false
|
||
})
|
||
```
|
||
|
||
**实测响应头**：
|
||
```
|
||
Set-Cookie: flamego_session=pqc9w5j3kq9thjxu; Path=/; HttpOnly; SameSite=Lax
|
||
```
|
||
`HttpOnly` 与 `SameSite=Lax` 有（库默认），**但 `Secure` 缺失**。虽然站点已下发 HSTS，但用户首次访问（HSTS 尚未落地、且未加入 preload 名单）时，主动 MITM 可诱导明文 HTTP 请求窃取会话 cookie。
|
||
|
||
**修复**：
|
||
```go
|
||
sessioner := session.Sessioner(session.Options{
|
||
Initer: initer,
|
||
Config: sessionStorage,
|
||
Cookie: session.CookieOptions{
|
||
HTTPOnly: true,
|
||
SameSite: http.SameSiteLaxMode,
|
||
Secure: strings.HasPrefix(conf.App.ExternalURL, "https"),
|
||
},
|
||
})
|
||
```
|
||
建议与 `securityHeaders()` 中已有的 HSTS 判断复用同一条件，保持 http 本地开发可用。
|
||
|
||
---
|
||
|
||
### 🟠 P1-3 无请求体总大小上限，可磁盘耗尽 DoS
|
||
|
||
**位置**：`internal/form/form.go:52`
|
||
|
||
```go
|
||
if err := r.ParseMultipartForm(10 * 1 << 20); err != nil { // 10 MiB
|
||
```
|
||
|
||
**关键误区澄清**：`10 MiB` 参数**只限制内存缓冲**，超出部分由 `mime/multipart` 自动落盘到临时文件，**总量无任何上限**；且全项目未使用 `http.MaxBytesReader`（已全局搜索确认）。
|
||
|
||
匿名可达的提问接口即接受 multipart，攻击者可持续 POST 超大 body 撑满磁盘。`ReadTimeout: 30s` 仅提供有限约束，高带宽下 30 秒足以写入数 GB。
|
||
|
||
> **附带澄清一个误报**：初步审计曾怀疑 `imgFile.Size`（`route/question/page.go:632`）取自客户端声明的 `Content-Length` 因而可伪造绕过 5MB 校验。**我编写测试实测否定了这一点** —— 构造声明 `Content-Length: 10` 而实际写入 100000 字节的 multipart part，`FileHeader.Size` 如实返回 `100000`。Go 的该字段反映真实字节数，**5MB 校验有效，此处无漏洞**。
|
||
|
||
**修复**（在 `form.Bind` 或最外层中间件统一加固）：
|
||
```go
|
||
const maxBody = 12 << 20 // 略高于业务上限 10MiB，留 multipart 边界开销
|
||
r.Body = http.MaxBytesReader(w, r.Body, maxBody)
|
||
if err := r.ParseMultipartForm(10 << 20); err != nil {
|
||
// 超限时返回 413 Request Entity Too Large
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 🟠 P1-4 IPv6 未按 `/64` 聚合，限流与自动封禁可被绕过
|
||
|
||
**位置**：`internal/shared/ip.go:105`、`internal/route/ratelimit.go:140`、`internal/security/autoban/autoban.go`
|
||
|
||
`ResolveClientIP` 返回完整 IPv6 地址，限流与 autoban 直接以其作 map key。单个住宅 IPv6 `/64` 前缀通常可用 2^64 个地址，攻击者每请求换一个源地址，则每个 key 仅命中 1 次，**阈值永远无法触发**；同时海量 key 会持续膨胀内存。
|
||
|
||
**实测补充**：IPv4 场景下限流工作正常 —— 连续 26 次请求精确地在第 21 次开始返回 `429`；伪造 `X-Forwarded-For` / `CF-Connecting-IP` / `X-Real-IP` **均未能绕过**（`trust_proxy` 关闭时确实忽略这些头，实现正确）。本条仅针对 IPv6 键粒度。
|
||
|
||
**修复**：
|
||
```go
|
||
// internal/shared/ip.go
|
||
func LimitKey(ip string) string {
|
||
p := net.ParseIP(ip)
|
||
if p == nil { return ip }
|
||
if v4 := p.To4(); v4 != nil { return v4.String() }
|
||
return p.Mask(net.CIDRMask(64, 128)).String() + "/64"
|
||
}
|
||
```
|
||
在限流与 autoban 取 key 处改用 `shared.LimitKey(ip)`；**IP 黑名单仍保持精确匹配**，不要套用聚合。
|
||
|
||
---
|
||
|
||
### 🟠 P1-5 限流 / 封禁状态仅存进程内存，多实例部署失效
|
||
|
||
**位置**：`internal/route/ratelimit.go:17`（`visitors` 为进程内 map）
|
||
|
||
限流与 autoban 状态均在单进程内存中，未落 Redis（尽管 Redis 已作为 session/cache 后端接入）。横向扩容到 N 个副本后，攻击者轮询各副本即可获得 **N 倍配额**；某副本封禁的 IP 打到其他副本仍被放行。
|
||
|
||
**修复**：迁移到 Redis 滑动窗口（`INCR` + `EXPIRE`，或 `SET NX EX`）；若坚持单副本部署，请在 README 与部署文档中明确标注「本服务不支持多副本水平扩展」，并配合宿主机 fail2ban 兜底。
|
||
|
||
---
|
||
|
||
## 三、P2 问题（改进建议）
|
||
|
||
| # | 问题 | 位置 | 说明与修复 |
|
||
|---|---|---|---|
|
||
| P2-1 | `safeURL` 允许协议相对 URL | `internal/template/markdown.go:246-255` | `strings.HasPrefix(lower, "/")` 会让 `//evil.com` 通过。**已写测试实测确认 `safeURL("//evil.com") == true`**。markdown 中 `[x](//evil.com)` 渲染为外链，可用于钓鱼。注意 `javascript:` 变体仍被正确拦截。修复：在白名单判断前先 `if strings.HasPrefix(lower, "//") { return false }` |
|
||
| P2-2 | 缺少 Content-Security-Policy | `internal/route/route.go:47-59` | 实测响应头确认无 CSP。其余安全头（X-Frame-Options / nosniff / Referrer-Policy / HSTS）齐备。因站内大量内联脚本，短期需保留 `'unsafe-inline'`，但限定 `script-src` 域名仍能显著收窄面 |
|
||
| P2-3 | `ProfileAPI` 返回完整 User 含 email | `route/user/profile.go:39-41` | `return ctx.JSON(ctx.User)`，而 `Email` 标签为 `json:"email"`（`users.go:82`）。虽为本人数据，仍违反最小暴露原则。建议返回裁剪后的 DTO |
|
||
| P2-4 | 提问删除级联未包事务 | `internal/db/questions.go:354-383` | 软删提问 → 删追问 → 删图片 → 回退配额，四步非原子，中途失败会留下孤儿记录与计数漂移。建议包入 `Transaction` |
|
||
| P2-5 | autoban 告警 goroutine 无 recover | `internal/security/autoban/autoban.go:172` | `go func(){ _ = b.sendAlert(...) }()` 一旦 panic 将终止整个进程。改用带 `recover` 的 `shared.SafeGo` |
|
||
| P2-6 | 登出未销毁会话 | `route/auth/logout.go:12` | 仅 `Session.Flush()` 清空数据，会话 ID 与 cookie 仍有效。建议改用 `Destroy()` 或登出后 `RegenerateID()` |
|
||
| P2-7 | 登录时序差异可枚举邮箱 | `internal/db/users.go:378` | 邮箱不存在时立即返回；存在但密码错时执行 bcrypt（约 50–100ms）。建议不存在时也比对一个固定 dummy hash 以对齐耗时（错误文案已正确统一） |
|
||
| P2-8 | 颜色字段仅校验长度未校验格式 | `internal/form/user.go:12-13` | `Usernamecolor` / `Introcolor` 仅 `minlen:7;maxlen:7`，可提交任意 7 字符。建议加 `Match(^#[0-9a-fA-F]{6}$)`。**注意：这不构成 XSS**，见第四节 |
|
||
| P2-9 | 会话有效期偏长 | `internal/route/route.go:97,108` | `Lifetime: 7 * 24h` 且无空闲超时，建议缩短并引入 idle timeout |
|
||
| P2-10 | `.bak` 文件被打包进二进制 | `templates/question/new-question-template.html.bak` | `//go:embed ... question ...` 会包含该文件（虽因 `.html` 后缀过滤不会被注册为模板）。建议删除或移出 embed 目录 |
|
||
| P2-11 | `app.sample.ini` 存在重复配置块 | `conf/app.sample.ini:82-86` 与 `89-93` | `image_endpoint` / `image_access_id` / `image_bucket` 等整组重复定义，会被静默覆盖为后者，易误导部署者。注释虽区分了「头像/背景」与「用户上传图片」，但键名完全相同 |
|
||
| P2-12 | `production` 默认 `false` | `internal/conf/static.go` + `route.go:63` | 缺省运行于 dev 模式可能暴露错误栈。建议默认按生产处理，或启动时强制校验该项 |
|
||
|
||
**附加工程建议**：`internal/db` 的 8 个测试因需要 `DB_TYPE`/`DB_USER` 等环境变量而全部失败（`internal/dbutil/testing.go:40`）。建议提供 `docker-compose.test.yml` 或 `Makefile` 目标一键拉起测试库，否则这部分测试在 CI 外基本不会被执行。
|
||
|
||
---
|
||
|
||
## 四、经验证排除的「疑似问题」（误报澄清）
|
||
|
||
审阅过程中有 3 项初步怀疑，我通过编写测试或实际请求**逐一验证后排除**，特此说明以免造成不必要的返工：
|
||
|
||
1. **模板 JS 上下文缺引号 ≠ 注入**
|
||
`templates/question/question-not-found.html:46-47` 写作 `style.color = {{.PageUser.Introcolor}};`（无引号），与 `list.html:132` 的 `"{{...}}"` 不一致，初判为 JS 表达式上下文注入。
|
||
**实测结论：安全。** Go `html/template` 在 JS 表达式上下文会自动 JSON 编码并补引号：输入 `0;alert(1` 输出 `"0;alert(1"`，输入 `</script>` 输出 `"\u003c/script\u003e"`。两种写法均安全，无需修改（仅风格不统一）。
|
||
|
||
2. **`multipart.FileHeader.Size` 不可伪造** —— 详见 P1-3 中的澄清说明。
|
||
|
||
3. **`service.Proxy` / `pixel.Proxy` 无 SSRF** —— 目标 Host 取自配置，用户仅控制路径且经 `path.Clean` 归一化，非用户可控目标。
|
||
|
||
---
|
||
|
||
## 五、经确认实现正确的部分
|
||
|
||
- **密码安全**：bcrypt(cost=10) + 每用户 salt，超 72 字节截断；旧 HMAC-SHA1 仅作兼容且登录时自动升级，旧路径用 `subtle.ConstantTimeCompare`
|
||
- **会话固定防护**：`login.go:67` 认证成功后先 `RegenerateID()` 再写入身份
|
||
- **密码重置**：`randstr.String(64)` 加密级随机，24h 过期，一次性消费，按 code 直查无枚举面
|
||
- **页面密码 / 提问密码**：均 bcrypt 存储与比较
|
||
- **验证码**：`crypto/rand` 生成，答案仅存服务端 session，5 分钟 TTL，一次性，尝试上限 5 次，`ConstantTimeCompare` 比对
|
||
- **SQL 注入**：`Raw`/`Exec` 仅用于固定 DDL；排序字段已归一化为 `ASC`/`DESC` 白名单；`LIKE` 有 `escapeLike` 转义 `% _ \`；`db.go` 的 `fmt.Sprintf` 仅拼接配置 schema 与硬编码表名
|
||
- **越权防护**：`getOwnedVote` 校验 `vote.UserID`；`Questioner` 中间件校验归属并注入 `canDelete`；chat 处理器校验 `OwnerID`/`AskerID`
|
||
- **SSRF**：**实测全部拦截** —— `127.0.0.1:6379`、`169.254.169.254`（云元数据）、`localhost`、`10.0.0.1`、`[::1]`、`example.com` 均返回 `403 domain not allowed`，`file://` 返回 `400 invalid url`。白名单默认为空即 fail-closed；`proxyImageDialContext` 在拨号时重新解析并固定公网 IP，彻底防 DNS rebinding；`CheckRedirect` 对每跳重复校验
|
||
- **开放重定向**：**实测全部拦截** —— `//evil.com`、`https://evil.com`、`\\evil.com`、`/\evil.com`、`////evil.com`、`/%09/evil.com`、`%2F%2Fevil.com` 一律回落到 `/`
|
||
- **XSS 主链路**：blackfriday 未开启 `HTML` 扩展（原始标签被转义），渲染后经白名单 sanitize + `on*` 属性剥离 + `safeURL`/`safeColorValue` 校验 + 文本节点二次转义；`toJSON` 依赖 `json.Marshal` 默认 HTML 转义
|
||
- **HTTP 方法约束**：`site-stats` 等只读端点对 `POST`/`PUT`/`DELETE` 均不执行；登出已正确改为 POST
|
||
- **中间件顺序**：CSRF 校验先于限流，无效 token 不消耗限流配额
|
||
- **并发安全**：`go test -race` 零 race；`rateLimiter.visitors`、`autoban.probes/banned`、`TTLCache.items`、`stats.cached`、`shared.App` 全部有锁保护；`captcha` 类型断言使用双返回值
|
||
- **数据一致性**：追问创建用 `clause.Locking` 行锁；会话消息计数为原子 `UPDATE ... WHERE count < ?` 且失败回滚；投票提交有复合唯一索引兜底；配额用 `gorm.Expr` 并防负数
|
||
- **敏感字段**：`Password`/`PagePassword`/`AskPassword`/`UID`/`BlockWords`/`FromIP`/`VoterIP` 均标 `json:"-"`；CSV/XLSX 导出有公式注入转义
|
||
- **超时配置**：`http.Server` 四类超时齐备；所有外部 HTTP 调用均设 `Timeout`
|
||
|
||
---
|
||
|
||
## 六、建议修复顺序
|
||
|
||
1. **P1-1 CSRF `SessionKey: "uid"`** —— 一行改动，收益最高，**建议补回归测试**
|
||
2. **P1-2 session cookie `Secure`** —— 一行改动
|
||
3. **P1-3 `http.MaxBytesReader`** —— 防匿名磁盘耗尽
|
||
4. **P1-4 IPv6 `/64` 聚合** —— 同时缓解内存膨胀
|
||
5. **P2-1 `safeURL` 拒绝 `//`** —— 一行改动
|
||
6. P1-5 与其余 P2 项按迭代节奏推进
|
||
|
||
---
|
||
|
||
## 七、修复状态（批次 A+B 已落地）
|
||
|
||
| 项 | 修复 | 回归测试 |
|
||
|----|------|----------|
|
||
| P1-1 CSRF `SessionKey: "uid"` | `route.go` 改为包级常量 `CSRFUserSessionKey = "uid"` | `internal/route/csrf_binding_test.go`：`TestCSRFTokenBoundToUserSession`（内存会话重放跨会话 token 复用 → 期望 400；SessionKey 误改回 `"userID"` 即失败） |
|
||
| P1-2 session cookie `Secure` | `route.go`：`Secure = strings.HasPrefix(ExternalURL,"https")` | （一行常量改动，行为随部署 `ExternalURL` 而定；HTTPS 部署下自动 `Secure`） |
|
||
| P1-3 请求体上限 | `form/form.go`：`http.MaxBytesReader` 封顶 12 MiB | （`internal/form` 既有测试保持通过） |
|
||
| P1-4 IPv6 `/64` 聚合 | `shared/ip.go` 新增 `LimitKey`；`ratelimit.go`/`autoban.go`/NotFound 统一套用 | `internal/shared/ip_test.go`：`TestLimitKey` + `TestLimitKeyIPv6SamePrefixCollapses` |
|
||
| P2-1 `safeURL` 拒绝 `//` | `markdown.go`：先拒协议相对 URL（`//`、`\\`、`/\`） | `internal/template/markdown_test.go`：`TestSafeURL` |
|
||
|
||
**验证**：`go build ./...` 与 `go vet ./...` 均干净；`internal/route`、`internal/shared`、`internal/template`、`internal/form` 测试全过（含上述 3 个新测试）。
|
||
**说明**：CSRF 跨会话拒绝（最关键动态发现）已由 DB 无关集成测试覆盖；本沙箱无 SQL DB（`db.Init` 启动连库），故未在此重启动服务做完整动态复验，需在用户可达 SQL DB 的测试环境下复跑 SSRF / 开放重定向 / 限流等探针。
|
||
|
||
---
|
||
|
||
*报告完毕。全程未读取 `conf/app.ini`；测试服务已停止，临时文件已清理；审阅期间在 `internal/template/` 临时创建的验证测试文件已删除，工作区无残留改动。*
|