diff --git a/SECURITY_TODO.md b/SECURITY_TODO.md
index 02d7f52..e54813a 100644
--- a/SECURITY_TODO.md
+++ b/SECURITY_TODO.md
@@ -95,21 +95,23 @@
- [x] goroutine 启动前同步提取 userID / ip / UA 为局部变量，`recordArticleView` 不再触碰 gin.Context 与 session
- **验证**: ✅ `go test -race ./...` 全绿
-### [ ] 20. 被禁用/锁定/删除用户的会话不失效（2026-08-27 复审新发现）
+### [x] 20. 被禁用/锁定/删除用户的会话不失效（2026-08-27 复审新发现）✅ 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`；其新评论不再自动通过
+ - [x] `AuthRequired(db)` 回库校验 `Status == StatusNormal` 且未软删，失败则清 session（保留 lang 与 csrf_token）并跳转 /login；session user_id 一律先断言为数值再入 GORM（呼应 #19）
+ - [x] SetUserContext：用户查询失败或非正常状态时 `is_logged_in` 置 false（评论自动通过随之失效，回落游客审核策略）
+- **验证**: ✅ `TestDisabledUserSessionInvalidated`（disabled/locked 旧 cookie → 302 /login）、`TestSoftDeletedUserSessionInvalidated`、`TestDisabledUserCommentsRequireApproval`（锁定后评论转 pending）
-### [ ] 21. 头像上传缺类别校验 + 可添加任意扩展名 → 存储型 XSS 链（2026-08-27 复审新发现）
+### [x] 21. 头像上传缺类别校验 + 可添加任意扩展名 → 存储型 XSS 链（2026-08-27 复审新发现）✅ 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` 类型 → 拒绝；正常图片仍成功
+ - [x] UploadAvatar / UpdateProfile 头像分支强制 `check.Type.Category == models.CategoryImage`，解码失败直接拒绝（不再回退存原始字节）
+ - [x] UpdateProfile 头像同样经 processAvatar 解码→缩放→JPEG 重编码，原始字节不再落盘
+ - [x] addUploadFileType 增加危险扩展黑名单（.html/.htm/.xhtml/.xht/.svg/.xml/.js/.mjs），拒绝添加并提示（settings_upload 页新增错误提示 + i18n）
+ - [x] 附带修复：processAvatar 依赖的 png/gif 解码器此前未注册（旧代码靠"失败回退"掩盖），补 blank import
+- **验证**: ✅ `TestAddUploadFileTypeRejectsDangerousExtensions`（6 组危险扩展拒绝 + .md 正常）、`TestUploadAvatarRejectsNonImage`（.html 拒绝 / 图片扩展名包 HTML 拒绝 / 正常 PNG 转存 .jpg）、`TestUpdateProfileAvatarRejectsNonImage`（表单头像同样拒绝 + 正常图片成功）
---
@@ -211,11 +213,9 @@
## 建议执行顺序
-P0/P1 原有 8 项及 P0 新发现 #18/#19 均已完成。剩余：
+P0/P1 原有 8 项及 P0/P1 新发现 #18–#21 均已完成。剩余：
-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：数据库文件移出存储根
+1. **#22 → #23 → #24**（校验类小改动，可合并一个 PR）
+2. **#9 → #10 → #25**（CDN 本地化、登录限速 + 计时抹平，同一主题）
+3. 其余 P2/P3（#11 配置权限、#12 首启弱凭据、#13 socket 权限、#14 magic bytes、#17 bcrypt cost）按迭代排入
+4. （可选）#18 方案 A：数据库文件移出存储根
diff --git a/handlers/profile.go b/handlers/profile.go
index 407c3dc..afe4736 100644
--- a/handlers/profile.go
+++ b/handlers/profile.go
@@ -19,6 +19,12 @@ import (
xdraw "golang.org/x/image/draw"
_ "golang.org/x/image/webp"
+ // Register the decoders processAvatar relies on. JPEG is registered by
+ // the image/jpeg import above; png/gif must be blank-imported or
+ // image.Decode would reject them.
+ _ "image/gif"
+ _ "image/png"
+
"go_blog/models"
)
@@ -94,7 +100,7 @@ func UpdateProfile(db *gorm.DB, storagePath string) gin.HandlerFunc {
// Validate against the platform upload policy (switch + type + size).
check := ValidateUpload(header)
- if !check.OK {
+ if !check.OK || check.Type.Category != models.CategoryImage {
session.Save()
reason := "?error=upload"
if !models.GetUploadConfig().Enabled {
@@ -109,6 +115,21 @@ func UpdateProfile(db *gorm.DB, storagePath string) gin.HandlerFunc {
// Determine file extension (validated to be in the whitelist).
ext := strings.ToLower(filepath.Ext(header.Filename))
+ // SECURITY (#21): decode and re-encode the avatar as a normalized
+ // JPEG instead of storing the original bytes — undecodable payloads
+ // (e.g. HTML disguised behind an image extension) are rejected.
+ imgBytes, err := io.ReadAll(file)
+ if err != nil {
+ c.Redirect(http.StatusFound, "/profile?error=upload")
+ return
+ }
+ processedBytes, finalExt, err := processAvatar(imgBytes, ext)
+ if err != nil {
+ session.Save()
+ c.Redirect(http.StatusFound, "/profile?error=upload")
+ return
+ }
+
// Save under storagePath/avatars/.
avatarDir := filepath.Join(storagePath, "avatars")
if err := os.MkdirAll(avatarDir, 0755); err != nil {
@@ -123,7 +144,7 @@ func UpdateProfile(db *gorm.DB, storagePath string) gin.HandlerFunc {
}
// Use user ID as filename base.
- savedName := fmt.Sprintf("%d%s", user.ID, ext)
+ savedName := fmt.Sprintf("%d%s", user.ID, finalExt)
savedPath := filepath.Join(avatarDir, savedName)
dst, err := os.Create(savedPath)
@@ -133,7 +154,7 @@ func UpdateProfile(db *gorm.DB, storagePath string) gin.HandlerFunc {
}
defer dst.Close()
- if _, err := io.Copy(dst, file); err != nil {
+ if _, err := dst.Write(processedBytes); err != nil {
c.Redirect(http.StatusFound, "/profile")
return
}
@@ -207,6 +228,14 @@ func UploadAvatar(db *gorm.DB, storagePath string) gin.HandlerFunc {
return
}
+ // SECURITY (#21): avatars must be an image-category type from the
+ // whitelist — the extension whitelist alone is admin-configurable and
+ // could otherwise admit active content into /uploads/avatars/.
+ if check.Type.Category != models.CategoryImage {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "file type not allowed"})
+ return
+ }
+
// Determine file extension (validated to be in the whitelist).
ext := strings.ToLower(filepath.Ext(header.Filename))
@@ -220,9 +249,10 @@ func UploadAvatar(db *gorm.DB, storagePath string) gin.HandlerFunc {
// Decode, resize, and re-encode the image.
processedBytes, finalExt, err := processAvatar(imgBytes, ext)
if err != nil {
- // Fall back to saving raw bytes if processing fails.
- processedBytes = imgBytes
- finalExt = ext
+ // SECURITY (#21): reject undecodable payloads outright — storing
+ // the raw bytes would let non-image content land in avatars/.
+ c.JSON(http.StatusBadRequest, gin.H{"error": "invalid image file"})
+ return
}
// Ensure avatar directory exists.
diff --git a/handlers/security_test.go b/handlers/security_test.go
index a5d34e7..952c898 100644
--- a/handlers/security_test.go
+++ b/handlers/security_test.go
@@ -43,7 +43,7 @@ func newSecurityTestEnv(t *testing.T) *securityTestEnv {
}
if err := db.AutoMigrate(&models.User{}, &models.Article{}, &models.Attachment{}, &models.SiteSetting{},
&models.UploadConfig{}, &models.UploadFileType{}, &models.CommentConfig{}, &models.NavLink{},
- &models.DownloadBaseURL{}); err != nil {
+ &models.DownloadBaseURL{}, &models.Comment{}); err != nil {
t.Fatalf("migrate: %v", err)
}
@@ -80,8 +80,9 @@ func newSecurityTestEnv(t *testing.T) *securityTestEnv {
r.GET("/login", LoginPage())
r.POST("/login", Login(db))
r.POST("/logout", Logout())
+ r.POST("/article/:slug/comments", PostComment(db))
- protected := r.Group("/my", middleware.AuthRequired())
+ protected := r.Group("/my", middleware.AuthRequired(db))
{
protected.POST("/articles/attachments", UploadAttachment(db, storageDir))
protected.POST("/articles/attachments/:id/delete", DeleteAttachment(db, storageDir))
@@ -92,8 +93,21 @@ func newSecurityTestEnv(t *testing.T) *securityTestEnv {
})
}
+ // Profile routes (avatar upload XSS-chain regression coverage, #21).
+ profile := r.Group("/profile", middleware.AuthRequired(db))
+ {
+ profile.POST("", UpdateProfile(db, storageDir))
+ profile.POST("/avatar", UploadAvatar(db, storageDir))
+ }
+
+ // Upload settings routes (dangerous-extension blacklist coverage, #21).
+ adminSettings := r.Group("/admin/settings", middleware.AuthRequired(db), middleware.AdminRequired(db))
+ {
+ adminSettings.POST("/upload", UploadSettingsSave(db))
+ }
+
// Admin user-management routes (SQL-injection regression coverage, #19).
- admin := r.Group("/admin", middleware.AuthRequired(), middleware.AdminRequired(db))
+ admin := r.Group("/admin", middleware.AuthRequired(db), middleware.AdminRequired(db))
{
admin.GET("/users/:id/edit", UserEditPage(db))
admin.POST("/users/:id/edit", UserUpdate(db))
diff --git a/handlers/session_upload_security_test.go b/handlers/session_upload_security_test.go
new file mode 100644
index 0000000..fd1ca8e
--- /dev/null
+++ b/handlers/session_upload_security_test.go
@@ -0,0 +1,295 @@
+package handlers
+
+import (
+ "bytes"
+ "fmt"
+ "image"
+ "image/color"
+ "image/png"
+ "mime/multipart"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "go_blog/models"
+)
+
+// seedUploadType inserts an upload file-type row directly in the DB and
+// reloads the config cache, simulating policy rows created before a fix.
+func seedUploadType(t *testing.T, e *securityTestEnv, ext, category string) {
+ t.Helper()
+ if err := e.db.Create(&models.UploadFileType{
+ Extension: ext, MimeType: "application/octet-stream", Category: category, Enabled: true,
+ }).Error; err != nil {
+ t.Fatalf("seed upload type %s: %v", ext, err)
+ }
+ models.LoadConfigCache(e.db)
+}
+
+// pngBytes renders a small valid PNG.
+func pngBytes(t *testing.T) []byte {
+ t.Helper()
+ img := image.NewRGBA(image.Rect(0, 0, 8, 8))
+ for x := 0; x < 8; x++ {
+ for y := 0; y < 8; y++ {
+ img.Set(x, y, color.RGBA{R: 0x33, G: 0x66, B: 0x99, A: 0xff})
+ }
+ }
+ var buf bytes.Buffer
+ if err := png.Encode(&buf, img); err != nil {
+ t.Fatalf("encode png: %v", err)
+ }
+ return buf.Bytes()
+}
+
+// multipartUpload posts a multipart form carrying one file field.
+func (e *securityTestEnv) multipartUpload(t *testing.T, path, cookie, csrfToken, fieldName, filename string, content []byte, fields map[string]string) *httptest.ResponseRecorder {
+ t.Helper()
+ var buf bytes.Buffer
+ mw := multipart.NewWriter(&buf)
+ mw.WriteField("_csrf", csrfToken)
+ for k, v := range fields {
+ mw.WriteField(k, v)
+ }
+ fw, _ := mw.CreateFormFile(fieldName, filename)
+ fw.Write(content)
+ mw.Close()
+ return e.do(http.MethodPost, path, cookie, strings.NewReader(buf.String()), mw.FormDataContentType())
+}
+
+func reloadAlice(t *testing.T, e *securityTestEnv) models.User {
+ t.Helper()
+ var alice models.User
+ if err := e.db.Where("username = ?", "alice").First(&alice).Error; err != nil {
+ t.Fatalf("load alice: %v", err)
+ }
+ return alice
+}
+
+// --- #20: stale sessions of disabled / locked / deleted users ---
+
+func TestDisabledUserSessionInvalidated(t *testing.T) {
+ e := newSecurityTestEnv(t)
+ aliceCookie := e.login(t, "alice")
+
+ // Sanity: the session works while the account is normal.
+ if w := e.do(http.MethodGet, "/my/whoami", aliceCookie, nil, ""); w.Code != http.StatusOK {
+ t.Fatalf("pre-disable /my/whoami: status=%d", w.Code)
+ }
+
+ for _, tc := range []struct {
+ name string
+ status int
+ }{
+ {"disabled", models.StatusDisabled},
+ {"locked", models.StatusLocked},
+ } {
+ e.db.Model(&models.User{}).Where("username = ?", "alice").Update("status", tc.status)
+ w := e.do(http.MethodGet, "/my/whoami", aliceCookie, nil, "")
+ if w.Code != http.StatusFound || w.Header().Get("Location") != "/login" {
+ t.Fatalf("%s user with stale cookie: status=%d location=%q, want 302 /login",
+ tc.name, w.Code, w.Header().Get("Location"))
+ }
+ // Restore so the next case starts from a normal account again.
+ e.db.Model(&models.User{}).Where("username = ?", "alice").Update("status", models.StatusNormal)
+ }
+}
+
+func TestSoftDeletedUserSessionInvalidated(t *testing.T) {
+ e := newSecurityTestEnv(t)
+ aliceCookie := e.login(t, "alice")
+
+ e.db.Where("username = ?", "alice").Delete(&models.User{})
+ w := e.do(http.MethodGet, "/my/whoami", aliceCookie, nil, "")
+ if w.Code != http.StatusFound || w.Header().Get("Location") != "/login" {
+ t.Fatalf("soft-deleted user with stale cookie: status=%d location=%q, want 302 /login",
+ w.Code, w.Header().Get("Location"))
+ }
+}
+
+func TestDisabledUserCommentsRequireApproval(t *testing.T) {
+ e := newSecurityTestEnv(t)
+ aliceCookie := e.login(t, "alice")
+ token := e.csrfTokenFor(t, aliceCookie)
+
+ // Guests must pass moderation for this scenario.
+ e.db.Model(&models.CommentConfig{}).Where("id = ?", 1).Update("guest_require_approval", true)
+ models.LoadConfigCache(e.db)
+
+ postComment := func() models.Comment {
+ t.Helper()
+ form := url.Values{}
+ form.Set("name", "alice")
+ form.Set("email", "alice@example.com")
+ form.Set("content", "comment body")
+ form.Set("_csrf", token)
+ w := e.do(http.MethodPost, "/article/alice-post/comments", aliceCookie,
+ strings.NewReader(form.Encode()), "application/x-www-form-urlencoded")
+ if w.Code != http.StatusFound {
+ t.Fatalf("POST comment: status=%d body=%s", w.Code, w.Body.String())
+ }
+ var cm models.Comment
+ if err := e.db.Last(&cm).Error; err != nil {
+ t.Fatalf("load comment: %v", err)
+ }
+ return cm
+ }
+
+ // Control: while alice is a normal user her comment is auto-approved.
+ if cm := postComment(); cm.Status != models.CommentApproved {
+ t.Fatalf("normal user comment status=%d, want approved", cm.Status)
+ }
+
+ // After being locked, her stale session no longer grants auto-approval:
+ // SetUserContext reports her as logged out, so the comment follows the
+ // guest moderation policy.
+ e.db.Model(&models.User{}).Where("username = ?", "alice").Update("status", models.StatusLocked)
+ if cm := postComment(); cm.Status != models.CommentPending {
+ t.Fatalf("locked user comment status=%d, want pending", cm.Status)
+ }
+}
+
+// --- #21: avatar upload XSS chain ---
+
+func TestAddUploadFileTypeRejectsDangerousExtensions(t *testing.T) {
+ e := newSecurityTestEnv(t)
+ admin := e.login(t, "admin")
+ token := e.csrfTokenFor(t, admin)
+
+ for _, ext := range []string{"html", ".htm", "SVG", "xhtml", ".xml", "js"} {
+ form := url.Values{}
+ form.Set("_csrf", token)
+ form.Set("action", "add_type")
+ form.Set("extension", ext)
+ form.Set("category", models.CategoryImage)
+ w := e.do(http.MethodPost, "/admin/settings/upload", admin,
+ strings.NewReader(form.Encode()), "application/x-www-form-urlencoded")
+ if w.Code != http.StatusFound || !strings.Contains(w.Header().Get("Location"), "error=dangerous_ext") {
+ t.Fatalf("add type %q: status=%d location=%q, want 302 with error=dangerous_ext",
+ ext, w.Code, w.Header().Get("Location"))
+ }
+ var count int64
+ normalized := strings.ToLower(ext)
+ if !strings.HasPrefix(normalized, ".") {
+ normalized = "." + normalized
+ }
+ e.db.Model(&models.UploadFileType{}).Where("extension = ?", normalized).Count(&count)
+ if count != 0 {
+ t.Fatalf("dangerous extension %q was persisted", ext)
+ }
+ }
+
+ // Control: a benign extension is still accepted.
+ form := url.Values{}
+ form.Set("_csrf", token)
+ form.Set("action", "add_type")
+ form.Set("extension", "md")
+ form.Set("category", models.CategoryDocument)
+ w := e.do(http.MethodPost, "/admin/settings/upload", admin,
+ strings.NewReader(form.Encode()), "application/x-www-form-urlencoded")
+ if w.Code != http.StatusFound || !strings.Contains(w.Header().Get("Location"), "saved=1") {
+ t.Fatalf("add benign type: status=%d location=%q", w.Code, w.Header().Get("Location"))
+ }
+ var count int64
+ e.db.Model(&models.UploadFileType{}).Where("extension = ?", ".md").Count(&count)
+ if count != 1 {
+ t.Fatalf("benign extension .md not created (count=%d)", count)
+ }
+}
+
+func TestUploadAvatarRejectsNonImage(t *testing.T) {
+ e := newSecurityTestEnv(t)
+ aliceCookie := e.login(t, "alice")
+ token := e.csrfTokenFor(t, aliceCookie)
+
+ htmlPayload := []byte("")
+
+ // Simulate a pre-existing dangerous type configured before the blacklist
+ // (defense in depth): the category check must reject it.
+ seedUploadType(t, e, ".html", models.CategoryOther)
+ w := e.multipartUpload(t, "/profile/avatar", aliceCookie, token, "avatar", "evil.html", htmlPayload, nil)
+ if w.Code != http.StatusBadRequest {
+ t.Fatalf("upload .html (other category): status=%d body=%s, want 400", w.Code, w.Body.String())
+ }
+
+ // Even a legacy .html row miscategorized as "image" is stopped by the
+ // decode step — raw bytes are never stored anymore.
+ seedUploadType(t, e, ".htm", models.CategoryImage)
+ w = e.multipartUpload(t, "/profile/avatar", aliceCookie, token, "avatar", "evil.htm", htmlPayload, nil)
+ if w.Code != http.StatusBadRequest {
+ t.Fatalf("upload .htm (image category): status=%d body=%s, want 400", w.Code, w.Body.String())
+ }
+
+ // HTML disguised behind a whitelisted image extension is likewise rejected.
+ seedUploadType(t, e, ".jpg", models.CategoryImage)
+ w = e.multipartUpload(t, "/profile/avatar", aliceCookie, token, "avatar", "x.jpg", htmlPayload, nil)
+ if w.Code != http.StatusBadRequest {
+ t.Fatalf("upload html as .jpg: status=%d body=%s, want 400", w.Code, w.Body.String())
+ }
+
+ // Nothing was stored and the avatar is unchanged.
+ if alice := reloadAlice(t, e); alice.Avatar != "" {
+ t.Fatalf("avatar unexpectedly set to %q", alice.Avatar)
+ }
+ if _, err := os.Stat(filepath.Join(e.storageDir, "avatars")); !os.IsNotExist(err) {
+ entries, _ := os.ReadDir(filepath.Join(e.storageDir, "avatars"))
+ for _, en := range entries {
+ t.Logf("avatars dir entry: %s", en.Name())
+ }
+ t.Fatal("avatar directory should not contain any file after rejected uploads")
+ }
+
+ // A real image is accepted, processed to a normalized JPEG.
+ seedUploadType(t, e, ".png", models.CategoryImage)
+ w = e.multipartUpload(t, "/profile/avatar", aliceCookie, token, "avatar", "me.png", pngBytes(t), nil)
+ if w.Code != http.StatusOK {
+ t.Fatalf("upload valid png: status=%d body=%s", w.Code, w.Body.String())
+ }
+ alice := reloadAlice(t, e)
+ if want := fmt.Sprintf("%d.jpg", alice.ID); alice.Avatar != want {
+ t.Fatalf("avatar = %q, want %q", alice.Avatar, want)
+ }
+ if _, err := os.Stat(filepath.Join(e.storageDir, "avatars", alice.Avatar)); err != nil {
+ t.Fatalf("processed avatar file missing: %v", err)
+ }
+}
+
+func TestUpdateProfileAvatarRejectsNonImage(t *testing.T) {
+ e := newSecurityTestEnv(t)
+ aliceCookie := e.login(t, "alice")
+ token := e.csrfTokenFor(t, aliceCookie)
+
+ seedUploadType(t, e, ".png", models.CategoryImage)
+ seedUploadType(t, e, ".html", models.CategoryImage) // legacy miscategorized row
+
+ // HTML behind a whitelisted extension must be rejected with the upload
+ // error redirect, and nothing may be written to avatars/.
+ htmlPayload := []byte("")
+ w := e.multipartUpload(t, "/profile", aliceCookie, token, "avatar", "evil.html", htmlPayload,
+ map[string]string{"display_name": "alice"})
+ if w.Code != http.StatusFound || !strings.Contains(w.Header().Get("Location"), "error=upload") {
+ t.Fatalf("update profile with html avatar: status=%d location=%q, want 302 error=upload",
+ w.Code, w.Header().Get("Location"))
+ }
+ if alice := reloadAlice(t, e); alice.Avatar != "" {
+ t.Fatalf("avatar unexpectedly set to %q", alice.Avatar)
+ }
+
+ // A real image goes through processing and is saved as JPEG.
+ w = e.multipartUpload(t, "/profile", aliceCookie, token, "avatar", "me.png", pngBytes(t),
+ map[string]string{"display_name": "alice"})
+ if w.Code != http.StatusFound || !strings.Contains(w.Header().Get("Location"), "saved=1") {
+ t.Fatalf("update profile with valid avatar: status=%d location=%q", w.Code, w.Header().Get("Location"))
+ }
+ alice := reloadAlice(t, e)
+ if want := fmt.Sprintf("%d.jpg", alice.ID); alice.Avatar != want {
+ t.Fatalf("avatar = %q, want %q", alice.Avatar, want)
+ }
+ if _, err := os.Stat(filepath.Join(e.storageDir, "avatars", alice.Avatar)); err != nil {
+ t.Fatalf("processed avatar file missing: %v", err)
+ }
+}
diff --git a/handlers/settings.go b/handlers/settings.go
index 0a61f88..de2b54b 100644
--- a/handlers/settings.go
+++ b/handlers/settings.go
@@ -225,6 +225,9 @@ func UploadSettingsPage(db *gorm.DB) gin.HandlerFunc {
if msg := c.Query("saved"); msg == "1" {
data["Success"] = tr["settings_saved"]
}
+ if msg := c.Query("error"); msg == "dangerous_ext" {
+ data["Error"] = tr["settings_upload_dangerous_ext"]
+ }
c.HTML(http.StatusOK, "settings_upload", data)
}
}
@@ -232,11 +235,14 @@ func UploadSettingsPage(db *gorm.DB) gin.HandlerFunc {
// UploadSettingsSave dispatches upload-config and file-type actions.
func UploadSettingsSave(db *gorm.DB) gin.HandlerFunc {
return func(c *gin.Context) {
+ redirect := "/admin/settings/upload?saved=1"
switch c.PostForm("action") {
case "save_config":
saveUploadConfig(db, c)
case "add_type":
- addUploadFileType(db, c)
+ if addUploadFileType(db, c) {
+ redirect = "/admin/settings/upload?error=dangerous_ext"
+ }
case "toggle_type":
toggleUploadFileType(db, c)
case "size_type":
@@ -245,7 +251,7 @@ func UploadSettingsSave(db *gorm.DB) gin.HandlerFunc {
deleteUploadFileType(db, c)
}
models.RefreshConfigCache(db)
- c.Redirect(http.StatusFound, "/admin/settings/upload?saved=1")
+ c.Redirect(http.StatusFound, redirect)
}
}
@@ -266,14 +272,28 @@ func saveUploadConfig(db *gorm.DB, c *gin.Context) {
db.Save(&u)
}
-func addUploadFileType(db *gorm.DB, c *gin.Context) {
+// dangerousUploadExtensions are never accepted as upload file types: files of
+// these extensions would be served same-origin from /uploads and can execute
+// active content (HTML/SVG/JS) in the site's origin, giving any logged-in
+// uploader a stored-XSS primitive (SECURITY_TODO #21).
+var dangerousUploadExtensions = map[string]bool{
+ ".html": true, ".htm": true, ".xhtml": true, ".xht": true,
+ ".svg": true, ".xml": true, ".js": true, ".mjs": true,
+}
+
+// addUploadFileType creates a new permitted file type. It reports whether the
+// extension was rejected as dangerous.
+func addUploadFileType(db *gorm.DB, c *gin.Context) bool {
ext := strings.ToLower(strings.TrimSpace(c.PostForm("extension")))
if ext == "" {
- return
+ return false
}
if !strings.HasPrefix(ext, ".") {
ext = "." + ext
}
+ if dangerousUploadExtensions[ext] {
+ return true
+ }
t := models.UploadFileType{
Extension: ext,
MimeType: strings.TrimSpace(c.PostForm("mime_type")),
@@ -287,6 +307,7 @@ func addUploadFileType(db *gorm.DB, c *gin.Context) {
}
// Ignore duplicate-extension errors silently.
db.Where("extension = ?", t.Extension).FirstOrCreate(&t)
+ return false
}
func toggleUploadFileType(db *gorm.DB, c *gin.Context) {
diff --git a/i18n/i18n.go b/i18n/i18n.go
index 1009526..3795200 100644
--- a/i18n/i18n.go
+++ b/i18n/i18n.go
@@ -254,6 +254,7 @@ var translations = map[Lang]map[string]string{
"settings_save": "Save",
"settings_upload_title": "Upload Settings",
"settings_upload_desc": "Attachment upload policy and permitted file types.",
+ "settings_upload_dangerous_ext": "This extension is not allowed: files of this type can execute active content in the site's origin.",
"settings_uploads_enabled":"Enable attachments",
"settings_default_size": "Default max size (MB)",
"settings_storage_dir": "Storage sub-directory",
@@ -672,6 +673,7 @@ var translations = map[Lang]map[string]string{
"settings_save": "保存",
"settings_upload_title": "上传设置",
"settings_upload_desc": "附件上传策略与允许的文件类型。",
+ "settings_upload_dangerous_ext": "不允许该扩展名：此类文件可在站点同源执行活动内容。",
"settings_uploads_enabled":"启用附件上传",
"settings_default_size": "默认最大大小（MB）",
"settings_storage_dir": "存储子目录",
diff --git a/main.go b/main.go
index 753edaa..58711eb 100644
--- a/main.go
+++ b/main.go
@@ -126,7 +126,7 @@ func main() {
// Protected admin routes (admin role only).
admin := router.Group("/admin")
- admin.Use(middleware.AuthRequired(), middleware.AdminRequired(db))
+ admin.Use(middleware.AuthRequired(db), middleware.AdminRequired(db))
{
admin.GET("", handlers.AdminDashboard(db))
admin.GET("/articles", handlers.ArticleListPage(db))
@@ -140,7 +140,7 @@ func main() {
// Protected admin comment management routes (admin role only).
comments := router.Group("/admin/comments")
- comments.Use(middleware.AuthRequired(), middleware.AdminRequired(db))
+ comments.Use(middleware.AuthRequired(db), middleware.AdminRequired(db))
{
comments.GET("", handlers.CommentListPage(db))
comments.POST("/:id/approve", handlers.CommentApprove(db))
@@ -150,7 +150,7 @@ func main() {
// Protected admin user-management routes (admin role only).
users := router.Group("/admin/users")
- users.Use(middleware.AuthRequired(), middleware.AdminRequired(db))
+ users.Use(middleware.AuthRequired(db), middleware.AdminRequired(db))
{
users.GET("", handlers.UserListPage(db))
users.GET("/new", handlers.UserCreatePage(db))
@@ -162,7 +162,7 @@ func main() {
// Protected article attachment routes (admin role only).
attachments := router.Group("/admin/articles")
- attachments.Use(middleware.AuthRequired(), middleware.AdminRequired(db))
+ attachments.Use(middleware.AuthRequired(db), middleware.AdminRequired(db))
{
attachments.POST("/attachments", handlers.UploadAttachment(db, cfg.Path))
attachments.POST("/attachments/:id/delete", handlers.DeleteAttachment(db, cfg.Path))
@@ -171,7 +171,7 @@ func main() {
// Protected admin settings routes (platform configuration).
settings := router.Group("/admin/settings")
- settings.Use(middleware.AuthRequired(), middleware.AdminRequired(db))
+ settings.Use(middleware.AuthRequired(db), middleware.AdminRequired(db))
{
settings.GET("/site", handlers.SiteSettingsPage(db))
settings.POST("/site", handlers.SiteSettingsSave(db, cfg.Path))
@@ -187,14 +187,14 @@ func main() {
// Protected admin analytics routes (reading statistics).
analytics := router.Group("/admin/analytics")
- analytics.Use(middleware.AuthRequired(), middleware.AdminRequired(db))
+ analytics.Use(middleware.AuthRequired(db), middleware.AdminRequired(db))
{
analytics.GET("/views", handlers.ViewAnalyticsPage(db))
}
// Protected profile routes.
profile := router.Group("/profile")
- profile.Use(middleware.AuthRequired())
+ profile.Use(middleware.AuthRequired(db))
{
profile.GET("", handlers.ProfilePage(db))
profile.POST("", handlers.UpdateProfile(db, cfg.Path))
@@ -203,7 +203,7 @@ func main() {
// Protected user article management routes (for non-admin users).
myArticles := router.Group("/my")
- myArticles.Use(middleware.AuthRequired())
+ myArticles.Use(middleware.AuthRequired(db))
{
myArticles.GET("/articles", handlers.MyArticlesPage(db))
myArticles.GET("/articles/new", handlers.MyArticleCreatePage(db))
@@ -215,7 +215,7 @@ func main() {
// Protected article attachment routes for user articles.
myAttachments := router.Group("/my/articles")
- myAttachments.Use(middleware.AuthRequired())
+ myAttachments.Use(middleware.AuthRequired(db))
{
myAttachments.POST("/attachments", handlers.UploadAttachment(db, cfg.Path))
myAttachments.POST("/attachments/:id/delete", handlers.DeleteAttachment(db, cfg.Path))
diff --git a/middleware/auth.go b/middleware/auth.go
index fd9c2ac..d2fcc9c 100644
--- a/middleware/auth.go
+++ b/middleware/auth.go
@@ -11,13 +11,63 @@ import (
"go_blog/models"
)
-// AuthRequired is middleware that protects routes. If the user is not logged in,
-// they are redirected to /login.
-func AuthRequired() gin.HandlerFunc {
+// sessionUserID extracts the logged-in user's numeric ID from the session,
+// defending against int/uint/int64/float64 storage. ok=false if absent or of
+// an unexpected type.
+func sessionUserID(session sessions.Session) (uint, bool) {
+ userID := session.Get("user_id")
+ if userID == nil {
+ return 0, false
+ }
+ switch v := userID.(type) {
+ case uint:
+ return v, true
+ case int:
+ return uint(v), true
+ case int64:
+ return uint(v), true
+ case float64:
+ return uint(v), true
+ default:
+ return 0, false
+ }
+}
+
+// clearUserSession drops the authentication state from a session, keeping only
+// the harmless UI preferences (language and CSRF token, mirroring the login
+// handler's rotation) so forms already rendered in other tabs stay valid.
+func clearUserSession(session sessions.Session) {
+ lang, _ := session.Get("lang").(string)
+ csrfTok, _ := session.Get(CSRFSessionKey).(string)
+ session.Clear()
+ if lang != "" {
+ session.Set("lang", lang)
+ }
+ if csrfTok != "" {
+ session.Set(CSRFSessionKey, csrfTok)
+ }
+ session.Save()
+}
+
+// AuthRequired is middleware that protects routes. If the user is not logged
+// in, they are redirected to /login. The session user is also re-validated
+// against the database on every request: an account that has since been
+// disabled, locked or soft-deleted loses access immediately instead of when
+// its cookie expires (SECURITY_TODO #20).
+func AuthRequired(db *gorm.DB) gin.HandlerFunc {
return func(c *gin.Context) {
session := sessions.Default(c)
- userID := session.Get("user_id")
- if userID == nil {
+ uid, ok := sessionUserID(session)
+ if !ok {
+ c.Redirect(http.StatusFound, "/login")
+ c.Abort()
+ return
+ }
+ var user models.User
+ if err := db.First(&user, uid).Error; err != nil || user.Status != models.StatusNormal {
+ // Account no longer usable — kill the session so the stale cookie
+ // cannot be replayed.
+ clearUserSession(session)
c.Redirect(http.StatusFound, "/login")
c.Abort()
return
@@ -27,19 +77,19 @@ func AuthRequired() gin.HandlerFunc {
}
// AdminRequired is middleware that restricts a route to admin-role users. It
-// must run after AuthRequired (which guarantees a session user exists). Non-admin
-// users are redirected back to the admin dashboard.
+// must run after AuthRequired (which guarantees a live, normal-status session
+// user). Non-admin users are redirected back to the admin dashboard.
func AdminRequired(db *gorm.DB) gin.HandlerFunc {
return func(c *gin.Context) {
session := sessions.Default(c)
- userID := session.Get("user_id")
- if userID == nil {
+ uid, ok := sessionUserID(session)
+ if !ok {
c.Redirect(http.StatusFound, "/login")
c.Abort()
return
}
var user models.User
- if err := db.First(&user, userID).Error; err != nil || user.Role != models.RoleAdmin {
+ if err := db.First(&user, uid).Error; err != nil || user.Role != models.RoleAdmin {
c.Redirect(http.StatusFound, "/admin")
c.Abort()
return
@@ -94,17 +144,20 @@ func SetUserContext(db *gorm.DB) gin.HandlerFunc {
c.Set("switch_lang", switchLang)
// --- Auth state ---
- userID := session.Get("user_id")
+ // The user is only considered logged in if the account still exists
+ // and is in normal status: a disabled/locked/soft-deleted account must
+ // not keep template-level privileges (e.g. comment auto-approval)
+ // after its session was invalidated (SECURITY_TODO #20).
isLoggedIn := false
var username string
var avatar string
var displayName string
var role string
- if userID != nil {
- isLoggedIn = true
+ if uid, ok := sessionUserID(session); ok {
var user models.User
- if err := db.First(&user, userID).Error; err == nil {
+ if err := db.First(&user, uid).Error; err == nil && user.Status == models.StatusNormal {
+ isLoggedIn = true
username = user.Username
avatar = user.Avatar
displayName = user.DisplayName
diff --git a/templates/admin/settings_upload.html b/templates/admin/settings_upload.html
index a6ae3aa..9953e83 100644
--- a/templates/admin/settings_upload.html
+++ b/templates/admin/settings_upload.html
@@ -14,6 +14,9 @@
{{if .Success}}
{{.Success}}
{{end}}
+ {{if .Error}}
+ {{.Error}}
+ {{end}}