fix: 修复 P0 复审漏洞 #18 #19(数据库泄露 + SQL 注入)
- #18 /uploads 改为白名单子目录挂载(attachments/avatars/logos + 配置的 存储目录),存储根不再整体暴露,blog.db 不可被下载;禁用目录列表, 拒绝 .. 穿越与反斜杠 - #19 admin 用户管理三个 handler 的路由参数先解析为数值(uintFormID), 非数值直接 302,消除 GORM First() 字符串条件 SQL 注入 - 新增回归测试 main_test.go(4 用例)与 TestAdminUserRoutesRejectNonNumericIDs (已变异验证:旧代码下注入用例失败) - SECURITY_TODO.md 勾选 #18/#19 并更新执行顺序
This commit is contained in:
5 files changed
+376
-20
No files matched your search
+74
-7
@@ -1,6 +1,7 @@
|
||||
# 安全修复 TODO
|
||||
|
||||
基于 2026-08-19 的安全审计(源码 + haibara.ai 线上验证)整理。
|
||||
2026-08-27 复审新增 #18–#25(标题标注"复审新发现")。
|
||||
按优先级排序,完成后勾选并标注日期。
|
||||
|
||||
---
|
||||
@@ -42,6 +43,24 @@
|
||||
- [x] 认证成功后先 `session.Clear()` 再写入 `user_id`/`username` 并 Save;保留 lang 与 csrf_token(避免多标签页已渲染表单失效)
|
||||
- **验证**: ✅ 登录前后 cookie 值不同,旧 cookie 无法访问受保护路由(`TestLoginRotatesSession`)
|
||||
|
||||
### [x] 18. SQLite 数据库文件可被公开下载(全库泄露)(2026-08-27 复审新发现)✅ 2026-08-27
|
||||
- **位置**: `main.go:72`(`router.Static("/uploads", cfg.Path)` 把存储根整体挂载为静态目录)、`models/db.go:36`(`blog.db` 就放在该目录下)
|
||||
- **问题**: 未认证即可 `GET /uploads/blog.db` 下载整库——含 bcrypt 密码哈希、用户邮箱、评论者 IP/邮箱、私密评论(IsPrivate)、浏览记录等。
|
||||
- **修复**(采用方案 B,白名单挂载):
|
||||
- [x] `main.go` 新增 `registerUploadRoutes`:只挂载 `attachments/`、`avatars/`、`logos/` 及当前配置的附件存储目录,存储根不再整体暴露
|
||||
- [x] 自定义 `serveUploadDir` handler:禁用目录列表(目录/`..`/`\` 一律 404),只服务具体文件
|
||||
- [x] `safeStorageDir`:storage_dir 含穿越/绝对路径/反斜杠时回退 `attachments`(为 #22 提供纵深防御)
|
||||
- [ ] (可选加固)方案 A:将数据库文件移出存储根,需迁移存量 blog.db,暂缓
|
||||
- **验证**: ✅ `main_test.go` 4 用例(blog.db 404 / 无目录列表 / 穿越失败 / 三目录正常服务、自定义 storage_dir、不安全回退、重名去重)
|
||||
|
||||
### [x] 19. GORM 字符串条件 SQL 注入(admin 用户管理 3 处)(2026-08-27 复审新发现)✅ 2026-08-27
|
||||
- **位置**: `handlers/admin_user.go:253、289、362`(`db.First(&user, id)`,id 为 `c.Param("id")` 字符串)
|
||||
- **问题**: GORM 对 `First(dest, s)`:s 为非数字字符串且无附加参数时按原始 SQL 条件拼入 WHERE(v1.31.1 `statement.go:293-303` 已核实)。`GET /admin/users/1 OR 1=1/edit` 即可注入;UserEditPage 为 GET 无 CSRF 拦截,可诱导已登录管理员点击链接触发盲注。
|
||||
- **修复**:
|
||||
- [x] UserEditPage / UserUpdate / UserDelete 三处:先 `uintFormID` 解析为数值,非数值(0)直接 302 回列表页;查询一律走主键参数化
|
||||
- [x] 表单 Action URL 由解析后的数值 ID 构造,不再回拼原始路由参数
|
||||
- **验证**: ✅ `TestAdminUserRoutesRejectNonNumericIDs`(5 组注入串 × GET/POST/DELETE 均被拒、数据零变更、合法 ID 不受影响);变异测试确认旧代码下注入用例失败("1 OR 1=1" 返回 200)
|
||||
|
||||
---
|
||||
|
||||
## P1 — 近期修复
|
||||
@@ -76,6 +95,22 @@
|
||||
- [x] goroutine 启动前同步提取 userID / ip / UA 为局部变量,`recordArticleView` 不再触碰 gin.Context 与 session
|
||||
- **验证**: ✅ `go test -race ./...` 全绿
|
||||
|
||||
### [ ] 20. 被禁用/锁定/删除用户的会话不失效(2026-08-27 复审新发现)
|
||||
- **位置**: `middleware/auth.go:16-27`(AuthRequired 只看 session 是否有 user_id,不回库校验)
|
||||
- **问题**: 登录时的状态检查(`handlers/auth.go:50`)只在登录瞬间生效。管理员禁用/锁定/软删用户后,其已持有的 cookie 在最长 24h 内仍完全可用:发评论自动 Approved、写文章、传附件;`SetUserContext` 对已软删用户仍置 `is_logged_in=true`。
|
||||
- **修复**:
|
||||
- [ ] AuthRequired 回库校验 `Status == StatusNormal` 且未软删,失败则清 session 并跳转 /login
|
||||
- [ ] SetUserContext:用户查询失败时 `is_logged_in` 置 false
|
||||
- **验证**: 禁用用户后用旧 cookie 访问 `/my/articles` → 302 `/login`;其新评论不再自动通过
|
||||
|
||||
### [ ] 21. 头像上传缺类别校验 + 可添加任意扩展名 → 存储型 XSS 链(2026-08-27 复审新发现)
|
||||
- **位置**: `handlers/profile.go:196-208`(UploadAvatar 未限制 image 类别)、`handlers/profile.go:223-226`(processAvatar 失败回退存原始字节)、`handlers/profile.go:96-107`(UpdateProfile 头像分支同样无类别校验、原样落盘)、`handlers/settings.go:269-290`(addUploadFileType 无危险扩展黑名单)
|
||||
- **问题**: logo/favicon 上传要求 `Category == image`(settings.go:113/153),但头像上传只查扩展名白名单且解码失败仍存原始文件;管理员又可在上传设置里添加任意扩展名(含 `.html`/`.svg`)。组合链:添加 `.html` 类型 → 任意登录用户以头像名义上传 HTML → 落在 `/uploads/avatars/` 同源可执行(CSP `script-src 'self' 'unsafe-inline'` 放行)。
|
||||
- **修复**:
|
||||
- [ ] UploadAvatar / UpdateProfile 头像分支强制 `check.Type.Category == models.CategoryImage`,解码失败直接拒绝(不回退存原始字节)
|
||||
- [ ] addUploadFileType 增加危险扩展黑名单(.html/.htm/.svg/.xhtml/.xml 等),拒绝添加
|
||||
- **验证**: 上传 `.html` 头像 → 拒绝;后台添加 `.html` 类型 → 拒绝;正常图片仍成功
|
||||
|
||||
---
|
||||
|
||||
## P2 — 计划修复
|
||||
@@ -110,6 +145,27 @@
|
||||
- **位置**: `install_linux.sh:80`
|
||||
- **修复**: `chmod 660` + `chown root:blog_go`(反向代理进程加入同组),避免本机任意用户绕过 Cloudflare 直连
|
||||
|
||||
### [ ] 22. storage_dir 路径穿越(2026-08-27 复审新发现)
|
||||
- **位置**: `handlers/settings.go:262-264`(任意 storage_dir 直接入库)、`handlers/attachment.go:21-27`(`filepath.Join` 不清洗 `../`)
|
||||
- **问题**: 管理员把 storage_dir 设为 `../../tmp` 类值后,附件上传/删除将发生在存储根之外(越界读写)。
|
||||
- **修复**:
|
||||
- [ ] 校验 storage_dir 为单个安全路径段(如 `^[A-Za-z0-9_-]+$`),否则拒绝保存
|
||||
- **验证**: 提交 `../evil` → 拒绝;`attachments` → 正常
|
||||
|
||||
### [ ] 23. 密码策略缺失(改密/管理员建号无最小长度)(2026-08-27 复审新发现)
|
||||
- **位置**: `handlers/profile.go:146-159`(UpdateProfile 改密)、`handlers/admin_user.go` UserCreate/UserUpdate(建号/重置密码)
|
||||
- **问题**: 注册要求密码 ≥6 位,但个人改密与管理员建号/重置密码均可设 1 位弱密码。
|
||||
- **修复**:
|
||||
- [ ] 抽公共 `validatePassword`,三处统一调用(最小长度与注册口径一致)
|
||||
- **验证**: 改密为 1 位 → 拒绝
|
||||
|
||||
### [ ] 24. 邮箱字段不校验格式(2026-08-27 复审新发现)
|
||||
- **位置**: `handlers/auth.go:130`(注册)、`handlers/profile.go:81-83`(改邮箱)、`handlers/admin_user.go`(建号/编辑)
|
||||
- **问题**: 仅评论处调用 `mail.ParseAddress`;注册/改资料/管理员建号均可写入非法邮箱(脏数据 + Gravatar 哈希异常)。
|
||||
- **修复**:
|
||||
- [ ] 抽公共 `validateEmail`,各处统一调用
|
||||
- **验证**: 注册/改邮箱提交 `abc` → 拒绝
|
||||
|
||||
---
|
||||
|
||||
## P3 — 低优先级 / 观察项
|
||||
@@ -132,11 +188,19 @@
|
||||
- **位置**: `models/user.go:41`(DefaultCost=10)
|
||||
- **修复**: 提升到 12;已有哈希在用户下次改密时自然升级
|
||||
|
||||
### [ ] 25. 登录计时侧信道(用户名枚举)(2026-08-27 复审新发现)
|
||||
- **位置**: `handlers/auth.go:39-47`
|
||||
- **问题**: 用户不存在时立即返回、不执行 bcrypt;密码错误时执行 bcrypt(~100ms)。响应时间差可用于枚举有效用户名,与未修复的 #10(无速率限制)叠加放大。
|
||||
- **修复**:
|
||||
- [ ] 用户不存在时也执行一次 dummy bcrypt 比较(对固定哑哈希),抹平时间差
|
||||
- [ ] 与 #10 的速率限制一并实施
|
||||
- **验证**: 大样本计时统计:两分支无显著差异
|
||||
|
||||
---
|
||||
|
||||
## 不需要修复(已确认安全)
|
||||
## 不需要修复(已确认安全,2026-08-27 复审复核仍成立)
|
||||
|
||||
- SQL 注入:全参数化查询(GORM)
|
||||
- SQL 注入:全参数化查询(GORM)——唯一例外见 #19(admin_user.go 3 处字符串条件)
|
||||
- XSS:html/template 自动转义 + 评论双防御(服务端 strip + DOMPurify)
|
||||
- 密码哈希:bcrypt
|
||||
- 附件路径穿越:SHA-256 内容寻址文件名
|
||||
@@ -147,8 +211,11 @@
|
||||
|
||||
## 建议执行顺序
|
||||
|
||||
1. **#1 → #4 → #5**(一次提交:会话安全三件套,改动小、风险低)
|
||||
2. **#3**(附件越权,纯 handler 层校验)
|
||||
3. **#2**(CSRF,涉及全站表单,改动面最大,单独一个 PR 充分回归)
|
||||
4. **#7 → #8 → #6**(IP/竞态/响应头)
|
||||
5. P2/P3 按迭代排入
|
||||
P0/P1 原有 8 项及 P0 新发现 #18/#19 均已完成。剩余:
|
||||
|
||||
1. **#20**(会话失效校验,middleware 单点改动)
|
||||
2. **#21**(头像/XSS 链:类别校验 + 扩展名黑名单)
|
||||
3. **#22 → #23 → #24**(校验类小改动,可合并一个 PR)
|
||||
4. **#9 → #10 → #25**(CDN 本地化、登录限速 + 计时抹平,同一主题)
|
||||
5. 其余 P2/P3(#11 配置权限、#12 首启弱凭据、#13 socket 权限、#14 magic bytes、#17 bcrypt cost)按迭代排入
|
||||
6. (可选)#18 方案 A:数据库文件移出存储根
|
||||
+27
-9
@@ -1,6 +1,7 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -248,7 +249,15 @@ func UserCreate(db *gorm.DB) gin.HandlerFunc {
|
||||
func UserEditPage(db *gorm.DB) gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
tr := getTr(c)
|
||||
id := c.Param("id")
|
||||
// SECURITY (#19): the route param must be parsed to a numeric id
|
||||
// before touching GORM — a raw string passed as the single cond to
|
||||
// First() is interpolated as a SQL WHERE clause.
|
||||
id := uintFormID(c.Param("id"))
|
||||
if id == 0 {
|
||||
c.Redirect(http.StatusFound, "/admin/users")
|
||||
return
|
||||
}
|
||||
|
||||
var user models.User
|
||||
if err := db.First(&user, id).Error; err != nil {
|
||||
c.Redirect(http.StatusFound, "/admin/users")
|
||||
@@ -268,7 +277,7 @@ func UserEditPage(db *gorm.DB) gin.HandlerFunc {
|
||||
Role: user.Role,
|
||||
Status: user.Status,
|
||||
IsEdit: true,
|
||||
Action: "/admin/users/" + id + "/edit",
|
||||
Action: fmt.Sprintf("/admin/users/%d/edit", user.ID),
|
||||
TitleText: tr["user_edit_title"],
|
||||
}, "")
|
||||
}
|
||||
@@ -278,15 +287,19 @@ func UserEditPage(db *gorm.DB) gin.HandlerFunc {
|
||||
func UserUpdate(db *gorm.DB) gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
tr := getTr(c)
|
||||
id := c.Param("id")
|
||||
f := parseUserForm(c)
|
||||
f.ID = uintFormID(id)
|
||||
// SECURITY (#19): reject non-numeric ids before touching GORM (see
|
||||
// UserEditPage).
|
||||
if f.ID == 0 {
|
||||
c.Redirect(http.StatusFound, "/admin/users")
|
||||
return
|
||||
}
|
||||
f.IsEdit = true
|
||||
f.Action = "/admin/users/" + id + "/edit"
|
||||
f.Action = fmt.Sprintf("/admin/users/%d/edit", f.ID)
|
||||
f.TitleText = tr["user_edit_title"]
|
||||
|
||||
var user models.User
|
||||
if err := db.First(&user, id).Error; err != nil {
|
||||
if err := db.First(&user, f.ID).Error; err != nil {
|
||||
c.Redirect(http.StatusFound, "/admin/users")
|
||||
return
|
||||
}
|
||||
@@ -354,12 +367,17 @@ func UserUpdate(db *gorm.DB) gin.HandlerFunc {
|
||||
// UserDelete soft-deletes a user, with self-protection and last-admin guards.
|
||||
func UserDelete(db *gorm.DB) gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
id := c.Param("id")
|
||||
targetID := uintFormID(id)
|
||||
// SECURITY (#19): reject non-numeric ids before touching GORM (see
|
||||
// UserEditPage).
|
||||
targetID := uintFormID(c.Param("id"))
|
||||
if targetID == 0 {
|
||||
c.Redirect(http.StatusFound, "/admin/users")
|
||||
return
|
||||
}
|
||||
currentID := userIDFromSession(c)
|
||||
|
||||
var user models.User
|
||||
if err := db.First(&user, id).Error; err != nil {
|
||||
if err := db.First(&user, targetID).Error; err != nil {
|
||||
c.Redirect(http.StatusFound, "/admin/users")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -92,6 +92,14 @@ func newSecurityTestEnv(t *testing.T) *securityTestEnv {
|
||||
})
|
||||
}
|
||||
|
||||
// Admin user-management routes (SQL-injection regression coverage, #19).
|
||||
admin := r.Group("/admin", middleware.AuthRequired(), middleware.AdminRequired(db))
|
||||
{
|
||||
admin.GET("/users/:id/edit", UserEditPage(db))
|
||||
admin.POST("/users/:id/edit", UserUpdate(db))
|
||||
admin.POST("/users/:id/delete", UserDelete(db))
|
||||
}
|
||||
|
||||
return &securityTestEnv{router: r, db: db, storageDir: storageDir}
|
||||
}
|
||||
|
||||
@@ -370,6 +378,80 @@ func TestAttachmentAdminOverride(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdminUserRoutesRejectNonNumericIDs(t *testing.T) {
|
||||
e := newSecurityTestEnv(t)
|
||||
admin := e.login(t, "admin")
|
||||
token := e.csrfTokenFor(t, admin)
|
||||
|
||||
var alice models.User
|
||||
if err := e.db.Where("username = ?", "alice").First(&alice).Error; err != nil {
|
||||
t.Fatalf("alice not found: %v", err)
|
||||
}
|
||||
|
||||
// Malicious :id values. Before the #19 fix, GORM interpolated a
|
||||
// non-numeric single string cond into the WHERE clause raw
|
||||
// (e.g. WHERE 1 OR 1=1).
|
||||
ids := []string{
|
||||
"1 OR 1=1",
|
||||
"1;--",
|
||||
"1) OR (1=1",
|
||||
"1 UNION SELECT 1",
|
||||
"alice",
|
||||
}
|
||||
|
||||
for _, id := range ids {
|
||||
// GET edit page must redirect instead of rendering a matched row.
|
||||
w := e.do(http.MethodGet, "/admin/users/"+url.PathEscape(id)+"/edit", admin, nil, "")
|
||||
if w.Code != http.StatusFound {
|
||||
t.Fatalf("GET edit with id %q: status = %d, want 302", id, w.Code)
|
||||
}
|
||||
if loc := w.Header().Get("Location"); loc != "/admin/users" {
|
||||
t.Fatalf("GET edit with id %q: location = %q, want /admin/users", id, loc)
|
||||
}
|
||||
|
||||
// POST update must not modify anything (attempt role escalation).
|
||||
form := url.Values{}
|
||||
form.Set("_csrf", token)
|
||||
form.Set("role", models.RoleAdmin)
|
||||
form.Set("status", "1")
|
||||
form.Set("display_name", "hacked")
|
||||
w = e.do(http.MethodPost, "/admin/users/"+url.PathEscape(id)+"/edit", admin,
|
||||
strings.NewReader(form.Encode()), "application/x-www-form-urlencoded")
|
||||
if w.Code != http.StatusFound || w.Header().Get("Location") != "/admin/users" {
|
||||
t.Fatalf("POST edit with id %q: status = %d, location = %q", id, w.Code, w.Header().Get("Location"))
|
||||
}
|
||||
|
||||
// POST delete must not delete anything.
|
||||
form = url.Values{}
|
||||
form.Set("_csrf", token)
|
||||
w = e.do(http.MethodPost, "/admin/users/"+url.PathEscape(id)+"/delete", admin,
|
||||
strings.NewReader(form.Encode()), "application/x-www-form-urlencoded")
|
||||
if w.Code != http.StatusFound {
|
||||
t.Fatalf("POST delete with id %q: status = %d, want 302", id, w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// No user was modified or removed by any of the payloads.
|
||||
var count int64
|
||||
e.db.Model(&models.User{}).Count(&count)
|
||||
if count != 3 {
|
||||
t.Fatalf("user count = %d, want 3 (injection removed rows)", count)
|
||||
}
|
||||
var check models.User
|
||||
if err := e.db.Where("username = ?", "alice").First(&check).Error; err != nil {
|
||||
t.Fatalf("alice gone: %v", err)
|
||||
}
|
||||
if check.Role != models.RoleAuthor || check.DisplayName != "alice" {
|
||||
t.Fatalf("alice modified via id injection: role=%q display=%q", check.Role, check.DisplayName)
|
||||
}
|
||||
|
||||
// Sanity: a valid numeric id still works.
|
||||
w := e.do(http.MethodGet, fmt.Sprintf("/admin/users/%d/edit", alice.ID), admin, nil, "")
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("GET edit with valid id: status = %d, want 200", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func userIDByUsername(t *testing.T, db *gorm.DB, username string) uint {
|
||||
t.Helper()
|
||||
var u models.User
|
||||
|
||||
@@ -9,6 +9,9 @@ import (
|
||||
"net"
|
||||
"net/http"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"github.com/gin-contrib/sessions"
|
||||
"github.com/gin-contrib/sessions/cookie"
|
||||
@@ -45,8 +48,8 @@ func main() {
|
||||
store := cookie.NewStore([]byte(cfg.Secret))
|
||||
store.Options(sessions.Options{
|
||||
Path: "/",
|
||||
MaxAge: 86400, // 24 hours
|
||||
HttpOnly: true, // prevent XSS access
|
||||
MaxAge: 86400, // 24 hours
|
||||
HttpOnly: true, // prevent XSS access
|
||||
SameSite: http.SameSiteLaxMode, // CSRF defense-in-depth; token check is the primary control
|
||||
// Secure is set per request (over HTTPS only) in the middleware below.
|
||||
})
|
||||
@@ -68,8 +71,11 @@ func main() {
|
||||
// 5. Load HTML templates.
|
||||
router.LoadHTMLGlob("templates/**/*.html")
|
||||
|
||||
// 6. Serve uploaded files (avatars etc.) from the storage path.
|
||||
router.Static("/uploads", cfg.Path)
|
||||
// 6. Serve uploaded files (avatars etc.) from the storage path. Only the
|
||||
// known upload subdirectories are exposed — never the storage root
|
||||
// itself, which also holds the SQLite database file: mounting the whole
|
||||
// root would let anyone download /uploads/blog.db (SECURITY_TODO #18).
|
||||
registerUploadRoutes(router.Group("/uploads"), cfg.Path, models.GetUploadConfig().StorageDir)
|
||||
|
||||
// 6b. Serve bundled static assets (embedded into the binary).
|
||||
staticFS, err := fs.Sub(staticFiles, "static")
|
||||
@@ -253,3 +259,57 @@ func main() {
|
||||
// Block forever.
|
||||
select {}
|
||||
}
|
||||
|
||||
// registerUploadRoutes exposes the public upload subdirectories under the
|
||||
// /uploads group: avatars, logos, and the configured attachment storage
|
||||
// directory (plus the default "attachments" for backward compatibility).
|
||||
// The storage root itself is never mounted — it also contains the SQLite
|
||||
// database file, which must not be downloadable (SECURITY_TODO #18).
|
||||
// Directory listing is disabled: only concrete files resolve.
|
||||
func registerUploadRoutes(g *gin.RouterGroup, storagePath, storageDir string) {
|
||||
dirs := []string{"attachments", "avatars", "logos"}
|
||||
if dir := safeStorageDir(storageDir); dir != "attachments" && dir != "avatars" && dir != "logos" {
|
||||
dirs = append(dirs, dir)
|
||||
}
|
||||
for _, d := range dirs {
|
||||
h := serveUploadDir(filepath.Join(storagePath, d))
|
||||
g.GET("/"+d+"/*file", h)
|
||||
g.HEAD("/"+d+"/*file", h)
|
||||
}
|
||||
}
|
||||
|
||||
// safeStorageDir clamps the configured attachment storage directory to a
|
||||
// safe relative path: non-empty, not absolute, and free of ".." or "\".
|
||||
// Anything unsafe falls back to the default "attachments" so a
|
||||
// misconfigured storage_dir cannot escape the storage root (defense in
|
||||
// depth for SECURITY_TODO #22).
|
||||
func safeStorageDir(dir string) string {
|
||||
const fallback = "attachments"
|
||||
if dir == "" {
|
||||
return fallback
|
||||
}
|
||||
cleaned := path.Clean(dir)
|
||||
if path.IsAbs(cleaned) || cleaned == "." ||
|
||||
strings.Contains(cleaned, "..") || strings.Contains(cleaned, "\\") {
|
||||
return fallback
|
||||
}
|
||||
return cleaned
|
||||
}
|
||||
|
||||
// serveUploadDir serves concrete files from one upload subdirectory.
|
||||
// Directory listings and traversal attempts are rejected with 404.
|
||||
func serveUploadDir(root string) gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
rel := c.Param("file") // always begins with "/"
|
||||
if strings.Contains(rel, "..") || strings.ContainsRune(rel, '\\') {
|
||||
c.Status(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
full := filepath.Join(root, rel)
|
||||
if st, err := os.Stat(full); err != nil || st.IsDir() {
|
||||
c.Status(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
c.File(full)
|
||||
}
|
||||
}
|
||||
+129
@@ -0,0 +1,129 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// newUploadsRouter builds a router with the production upload routes over a
|
||||
// temp storage root that mirrors the real layout: the SQLite database file
|
||||
// lives in the root itself, uploads live in subdirectories.
|
||||
func newUploadsRouter(t *testing.T, storageDir string) (*gin.Engine, string) {
|
||||
t.Helper()
|
||||
gin.SetMode(gin.TestMode)
|
||||
|
||||
root := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(root, "blog.db"), []byte("fake sqlite"), 0644); err != nil {
|
||||
t.Fatalf("seed blog.db: %v", err)
|
||||
}
|
||||
|
||||
r := gin.New()
|
||||
registerUploadRoutes(r.Group("/uploads"), root, storageDir)
|
||||
return r, root
|
||||
}
|
||||
|
||||
func doGet(t *testing.T, r *gin.Engine, path string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
w := httptest.NewRecorder()
|
||||
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil))
|
||||
return w
|
||||
}
|
||||
|
||||
func seedUploadFile(t *testing.T, root, sub, name string) {
|
||||
t.Helper()
|
||||
dir := filepath.Join(root, sub)
|
||||
if err := os.MkdirAll(dir, 0755); err != nil {
|
||||
t.Fatalf("mkdir %s: %v", dir, err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(dir, name), []byte("content"), 0644); err != nil {
|
||||
t.Fatalf("seed %s: %v", name, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadsWhitelistHidesStorageRoot(t *testing.T) {
|
||||
r, root := newUploadsRouter(t, "")
|
||||
for _, sub := range []string{"attachments", "avatars", "logos"} {
|
||||
seedUploadFile(t, root, sub, "file.txt")
|
||||
}
|
||||
|
||||
// The database file in the storage root must not be downloadable.
|
||||
if w := doGet(t, r, "/uploads/blog.db"); w.Code != http.StatusNotFound {
|
||||
t.Fatalf("GET /uploads/blog.db = %d, want 404 (database leak)", w.Code)
|
||||
}
|
||||
|
||||
// No directory listing anywhere.
|
||||
for _, p := range []string{
|
||||
"/uploads", "/uploads/",
|
||||
"/uploads/attachments/", "/uploads/avatars/", "/uploads/logos/",
|
||||
} {
|
||||
if w := doGet(t, r, p); w.Code != http.StatusNotFound {
|
||||
t.Fatalf("GET %s = %d, want 404 (no directory listing)", p, w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// Traversal attempts must not escape the subdirectory.
|
||||
for _, p := range []string{
|
||||
"/uploads/attachments/../blog.db",
|
||||
"/uploads/attachments/..%2f..%2fblog.db",
|
||||
"/uploads/attachments/%2e%2e/blog.db",
|
||||
} {
|
||||
if w := doGet(t, r, p); w.Code == http.StatusOK {
|
||||
t.Fatalf("GET %s = %d, want non-200 (traversal)", p, w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// Files in the whitelisted subdirectories are still served.
|
||||
for _, p := range []string{
|
||||
"/uploads/attachments/file.txt",
|
||||
"/uploads/avatars/file.txt",
|
||||
"/uploads/logos/file.txt",
|
||||
} {
|
||||
if w := doGet(t, r, p); w.Code != http.StatusOK {
|
||||
t.Fatalf("GET %s = %d, want 200", p, w.Code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadsWhitelistCustomStorageDir(t *testing.T) {
|
||||
r, root := newUploadsRouter(t, "files")
|
||||
seedUploadFile(t, root, "files", "a.bin")
|
||||
|
||||
if w := doGet(t, r, "/uploads/files/a.bin"); w.Code != http.StatusOK {
|
||||
t.Fatalf("GET /uploads/files/a.bin = %d, want 200", w.Code)
|
||||
}
|
||||
// The default dir stays mounted for backward compatibility.
|
||||
seedUploadFile(t, root, "attachments", "old.txt")
|
||||
if w := doGet(t, r, "/uploads/attachments/old.txt"); w.Code != http.StatusOK {
|
||||
t.Fatalf("GET /uploads/attachments/old.txt = %d, want 200", w.Code)
|
||||
}
|
||||
if w := doGet(t, r, "/uploads/blog.db"); w.Code != http.StatusNotFound {
|
||||
t.Fatalf("GET /uploads/blog.db = %d, want 404", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadsWhitelistUnsafeStorageDirFallsBack(t *testing.T) {
|
||||
for _, dir := range []string{"../evil", "/etc", "..", "a/../../b", "."} {
|
||||
r, root := newUploadsRouter(t, dir)
|
||||
seedUploadFile(t, root, "attachments", "file.txt")
|
||||
if w := doGet(t, r, "/uploads/attachments/file.txt"); w.Code != http.StatusOK {
|
||||
t.Fatalf("storage dir %q: fallback mount broken: %d", dir, w.Code)
|
||||
}
|
||||
if w := doGet(t, r, "/uploads/blog.db"); w.Code != http.StatusNotFound {
|
||||
t.Fatalf("storage dir %q: /uploads/blog.db = %d, want 404", dir, w.Code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadsWhitelistStorageDirDedup(t *testing.T) {
|
||||
// A storage dir equal to a known dir must not panic on duplicate routes.
|
||||
r, root := newUploadsRouter(t, "avatars")
|
||||
seedUploadFile(t, root, "avatars", "me.jpg")
|
||||
if w := doGet(t, r, "/uploads/avatars/me.jpg"); w.Code != http.StatusOK {
|
||||
t.Fatalf("GET /uploads/avatars/me.jpg = %d, want 200", w.Code)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user