diff --git a/SECURITY_TODO.md b/SECURITY_TODO.md index 85f360a..61cf62f 100644 --- a/SECURITY_TODO.md +++ b/SECURITY_TODO.md @@ -249,13 +249,13 @@ - [x] 用户不存在时也执行一次 dummy bcrypt 比较（包级预生成哑哈希），抹平时间差；两分支均记录失败计数 - **验证**: ✅ 结构保证两分支均执行一次 bcrypt（`TestLoginTimingDoesNotRevealUser` 断言未知用户分支进入 Fail）；大样本计时统计属人工运维验证，逻辑上两分支 B 树一致 -### [ ] 32. 零碎加固（2026-08-27 API 化复审新发现） +### [x] 32. 零碎加固（2026-08-27 API 化复审新发现）✅ 2026-08-27 - **位置**: 多处 - **问题与修复**: - - [ ] `admin_user.go` UserUpdate：`status` 无枚举校验，可存任意 int（如 99）——限定 {0,1,2,3}，非法 400 - - [ ] `attachment.go` parseUintParam / parseUintForm：`Sscanf("%d")` 会把 `"5abc"` 宽松解析为 5——改 `strconv.ParseUint` 严格拒绝（无注入风险，值已为数值类型，仅严谨性） - - [ ] `settings.go` dangerousUploadExtensions：补充 `.xsl` / `.xslt` / `.shtml`（nosniff 已兜底，仅完整性） -- **验证**: [ ] 表驱动测试：非法 status → 400；`"5abc"` 形式的 id → 拒绝 + - [x] `admin_user.go` UserCreate/UserUpdate：`status` 无枚举校验，可存任意 int（如 99）——新增 `validUserStatus` 限定 {0,1,2,3}，非法 400（i18n `user_status_invalid` 中英新增） + - [x] `attachment.go` parseUintParam / parseUintForm：`Sscanf("%d")` 会把 `"5abc"` 宽松解析为 5——改 `strconv.ParseUint` 严格拒绝（无注入风险，值已为数值类型，仅严谨性） + - [x] `settings.go` dangerousUploadExtensions：补充 `.xsl` / `.xslt` / `.shtml`（nosniff 已兜底，仅完整性） +- **验证**: ✅ `TestUserStatusEnumRejected`（-1/99 → 400 且数据不变；锁定/禁用/正常逐一生效）、`TestParseUintStrict`（路由参数与表单字段的 "5abc"/溢出值拒绝，合法值正常）、`TestAddUploadFileTypeRejectsDangerousExtensions` 扩展用例（.xsl/.xslt/.shtml 拒绝 + 良性 .md 通过） --- @@ -276,13 +276,12 @@ ## 建议执行顺序 -#1–#25 及 #26 已修复并验证。剩余待办 #27–#32，建议顺序： - +**#1–#32 全部修复并验证完毕（2026-08-27）。** 实施顺序： 1. ~~#26 请求体大小限制~~ ✅ 2026-08-27 -2. **#27/#28 注册与评论限流**（P1，可与 #26 的中间件基建衔接实施） -3. **#29 favicon/logo 魔数校验**、**#30 最后管理员事务化**（P2，各自独立小改） -4. **#31 置顶权限**需先确认产品意图（作者可置顶是否预期）再决定修否 -5. **#32 零碎项**随手修 +2. ~~#27/#28 注册与评论限流~~ ✅ 2026-08-27（新增 `handlers/rate_limit.go` 固定窗口限流器，可与 #26 的中间件基建衔接） +3. ~~#29 favicon/logo 魔数校验~~、~~#30 最后管理员事务化~~ ✅ 2026-08-27（各自独立小改） +4. ~~#31 置顶权限~~ ✅ 2026-08-27（确认"作者可置顶"非产品预期，按收紧方案实施：作者 create/update 忽略 is_top，表单移除复选框；管理员授权置顶不因作者编辑丢失） +5. ~~#32 零碎项~~ ✅ 2026-08-27（status 枚举校验 / 严格 uint 解析 / 危险扩展名补充） 历史遗留观察项（不阻塞）： diff --git a/handlers/admin_user.go b/handlers/admin_user.go index 4ff3371..b05bc08 100644 --- a/handlers/admin_user.go +++ b/handlers/admin_user.go @@ -72,6 +72,18 @@ func uintFormID(s string) uint { return uint(n) } +// validUserStatus 报告后台用户表单提交的状态值是否属于枚举 +// {StatusDisabled(0), StatusNormal(1), StatusLocked(2), StatusUnactivated(3)} +// （SECURITY_TODO #32）。此前 status 无枚举校验，可写入任意 int +// （如 99），产生界面无法解释的状态。 +func validUserStatus(s int) bool { + switch s { + case models.StatusDisabled, models.StatusNormal, models.StatusLocked, models.StatusUnactivated: + return true + } + return false +} + // errLastAdminRemoval 与 lastAdminMu 实现"最后管理员"防线 // （SECURITY_TODO #30）。原实现先计数再写入，两者之间无原子性： // 两个并发的"降级/删除倒数第二位管理员"请求可同时通过检查， @@ -230,6 +242,11 @@ func UserCreate(db *gorm.DB) gin.HandlerFunc { APIError(c, http.StatusBadRequest, "user_username_required") return } + // SECURITY_TODO #32：status 必须属于枚举 {0,1,2,3}。 + if !validUserStatus(f.Status) { + APIError(c, http.StatusBadRequest, "user_status_invalid") + return + } if f.Password == "" { APIError(c, http.StatusBadRequest, "user_password_required") return @@ -328,6 +345,11 @@ func UserUpdate(db *gorm.DB) gin.HandlerFunc { APIError(c, http.StatusBadRequest, "api_invalid_request") return } + // SECURITY_TODO #32：status 必须属于枚举 {0,1,2,3}。 + if !validUserStatus(f.Status) { + APIError(c, http.StatusBadRequest, "user_status_invalid") + return + } var user models.User if err := db.First(&user, f.ID).Error; err != nil { diff --git a/handlers/attachment.go b/handlers/attachment.go index 9947138..f849c81 100644 --- a/handlers/attachment.go +++ b/handlers/attachment.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "path/filepath" + "strconv" "strings" "github.com/gin-gonic/gin" @@ -270,20 +271,26 @@ func BindPendingAttachments(db *gorm.DB, token string, articleID uint) error { // ---------------- 辅助函数 ---------------- -// parseUintForm 解析 uint 表单字段，容忍空/非法输入。 -func parseUintForm(c *gin.Context, field string) uint { - v := strings.TrimSpace(c.PostForm(field)) - if v == "" { +// parseUintStrict 严格解析十进制 uint：空值、非数字、前缀数字（"5abc"） +// 与超出范围的值一律返回 0（SECURITY_TODO #32——旧的 Sscanf("%d") 会把 +// "5abc" 宽松解析为 5，掩盖非法输入）。 +func parseUintStrict(s string) uint { + if s == "" { return 0 } - var n uint - _, _ = fmt.Sscanf(v, "%d", &n) - return n + n, err := strconv.ParseUint(s, 10, 64) + if err != nil || n > uint64(^uint(0)) { + return 0 + } + return uint(n) } -// parseUintParam 解析 uint 路由参数。 -func parseUintParam(c *gin.Context, name string) uint { - var n uint - _, _ = fmt.Sscanf(c.Param(name), "%d", &n) - return n +// parseUintForm 解析 uint 表单字段（严格；空/非法输入返回 0）。 +func parseUintForm(c *gin.Context, field string) uint { + return parseUintStrict(strings.TrimSpace(c.PostForm(field))) +} + +// parseUintParam 解析 uint 路由参数（严格；空/非法输入返回 0）。 +func parseUintParam(c *gin.Context, name string) uint { + return parseUintStrict(c.Param(name)) } diff --git a/handlers/misc_hardening_test.go b/handlers/misc_hardening_test.go new file mode 100644 index 0000000..84ee74a --- /dev/null +++ b/handlers/misc_hardening_test.go @@ -0,0 +1,92 @@ +package handlers + +import ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" + + "go_blog/models" +) + +// TestUserStatusEnumRejected 覆盖 SECURITY_TODO #32 第一项： +// UserCreate/UserUpdate 的 status 仅接受枚举 {0,1,2,3}，非法值返回 400 +// 且数据不变；合法枚举值正常生效。 +func TestUserStatusEnumRejected(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("load alice: %v", err) + } + + for _, bad := range []int{-1, 99} { + w := postJSON(e, http.MethodPut, fmt.Sprintf("/api/admin/users/%d", alice.ID), admin, token, + gin.H{"username": "alice", "email": "alice@example.com", + "role": models.RoleAuthor, "status": bad}) + if w.Code != http.StatusBadRequest || respCode(w) != "user_status_invalid" { + t.Fatalf("status %d: code = %d/%q, want 400/user_status_invalid", bad, w.Code, respCode(w)) + } + } + var reloaded models.User + if err := e.db.First(&reloaded, alice.ID).Error; err != nil { + t.Fatalf("reload alice: %v", err) + } + if reloaded.Status != models.StatusNormal { + t.Fatalf("invalid status persisted: status = %d", reloaded.Status) + } + + // 合法枚举值（锁定 → 禁用 → 恢复正常）逐个生效。 + for _, want := range []int{models.StatusLocked, models.StatusDisabled, models.StatusNormal} { + w := postJSON(e, http.MethodPut, fmt.Sprintf("/api/admin/users/%d", alice.ID), admin, token, + gin.H{"username": "alice", "email": "alice@example.com", + "role": models.RoleAuthor, "status": want}) + if w.Code != http.StatusOK || !respOK(w) { + t.Fatalf("status %d: code = %d, body %s", want, w.Code, w.Body.String()) + } + if err := e.db.First(&reloaded, alice.ID).Error; err != nil { + t.Fatalf("reload alice: %v", err) + } + if reloaded.Status != want { + t.Fatalf("status = %d, want %d", reloaded.Status, want) + } + } +} + +// TestParseUintStrict 覆盖 SECURITY_TODO #32 第二项： +// parseUintParam/parseUintForm 必须严格拒绝 "5abc" 一类的前缀数字输入 +// （旧的 fmt.Sscanf("%d") 会宽松解析为 5）。 +func TestParseUintStrict(t *testing.T) { + // 路由参数。 + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Params = gin.Params{{Key: "id", Value: "5abc"}} + if got := parseUintParam(c, "id"); got != 0 { + t.Fatalf("parseUintParam(5abc) = %d, want 0", got) + } + c.Params = gin.Params{{Key: "id", Value: "42"}} + if got := parseUintParam(c, "id"); got != 42 { + t.Fatalf("parseUintParam(42) = %d, want 42", got) + } + c.Params = gin.Params{{Key: "id", Value: "18446744073709551616"}} // > uint64 + if got := parseUintParam(c, "id"); got != 0 { + t.Fatalf("parseUintParam(overflow) = %d, want 0", got) + } + + // 表单字段（每个用例独立 context，避免 gin 的 form 缓存干扰）。 + formCtx := func(body string) *gin.Context { + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Request = httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + c.Request.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return c + } + if got := parseUintForm(formCtx("article_id=5abc"), "article_id"); got != 0 { + t.Fatalf("parseUintForm(article_id=5abc) = %d, want 0", got) + } + if got := parseUintForm(formCtx("article_id=7"), "article_id"); got != 7 { + t.Fatalf("parseUintForm(article_id=7) = %d, want 7", got) + } +} diff --git a/handlers/session_upload_security_test.go b/handlers/session_upload_security_test.go index 749ec49..cccde78 100644 --- a/handlers/session_upload_security_test.go +++ b/handlers/session_upload_security_test.go @@ -159,7 +159,8 @@ func TestAddUploadFileTypeRejectsDangerousExtensions(t *testing.T) { admin := e.login(t, "admin") token := e.csrfTokenFor(t, admin) - for _, ext := range []string{"html", ".htm", "SVG", "xhtml", ".xml", "js"} { + // SECURITY_TODO #32：黑名单补充 .xsl/.xslt/.shtml 后并入同一用例。 + for _, ext := range []string{"html", ".htm", "SVG", "xhtml", ".xml", "js", ".xsl", ".xslt", ".shtml"} { w := postJSON(e, http.MethodPost, "/api/admin/settings/upload", admin, token, gin.H{"action": "add_type", "extension": ext, "category": models.CategoryImage}) if w.Code != http.StatusBadRequest || respCode(w) != "settings_upload_dangerous_ext" { diff --git a/handlers/settings.go b/handlers/settings.go index e21e051..06a1eda 100644 --- a/handlers/settings.go +++ b/handlers/settings.go @@ -398,6 +398,9 @@ func saveUploadConfig(db *gorm.DB, req uploadSettingsRequest, updatedBy uint) bo var dangerousUploadExtensions = map[string]bool{ ".html": true, ".htm": true, ".xhtml": true, ".xht": true, ".svg": true, ".xml": true, ".js": true, ".mjs": true, + // SECURITY_TODO #32：补充服务器端处理型扩展名（XSLT 可内嵌脚本、 + // SSI 可包含文件），nosniff 已兜底，此处仅完整性。 + ".xsl": true, ".xslt": true, ".shtml": true, } // addUploadFileType 创建新的允许文件类型。报告扩展名是否因危险而被拒绝。 diff --git a/i18n/i18n.go b/i18n/i18n.go index c84618a..7112455 100644 --- a/i18n/i18n.go +++ b/i18n/i18n.go @@ -454,6 +454,7 @@ var translations = map[Lang]map[string]string{ "register_locked": "Too many registration attempts from your address. Please try again later.", "comments_locked": "Too many comments from your address. Please wait a moment and try again.", "settings_upload_bad_content": "File content does not match its declared type.", + "user_status_invalid": "Invalid user status.", "user_not_found": "User not found.", }, ZH: { @@ -894,6 +895,7 @@ var translations = map[Lang]map[string]string{ "register_locked": "来自该地址的注册次数过多，请稍后再试。", "comments_locked": "评论提交过于频繁，请稍后再试。", "settings_upload_bad_content": "文件内容与声明类型不匹配。", + "user_status_invalid": "无效的用户状态。", "user_not_found": "用户不存在。", }, }