From 8ea4a623a9bcecd62a153293f8fbef1ab5001129 Mon Sep 17 00:00:00 2001 From: kevin Date: Wed, 19 Aug 2026 16:56:23 +0800 Subject: [PATCH] =?UTF-8?q?fix(security):=20=E4=BF=AE=E5=A4=8D=20P3=20?= =?UTF-8?q?=E4=BD=8E=E5=8D=B1=E9=A1=B9=EF=BC=88=E5=BC=80=E6=94=BE=E9=87=8D?= =?UTF-8?q?=E5=AE=9A=E5=90=91/=E9=85=8D=E9=A2=9DTOCTOU/safeJS/=E4=BC=9A?= =?UTF-8?q?=E8=AF=9D=E6=B2=BB=E7=90=86=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Referer 开放重定向:safeRedirectPath 仅放行同站相对路径, 外部 URL/协议跳转一律回退 /inbox - 发信配额 TOCTOU:新增 TryReserveQuota 原子预扣 (UPDATE ... WHERE used_bytes + n <= quota_bytes),超配额即拒发; 附件保存失败按大小补偿回退 - 移除危险模板函数 safeHTML/safeJS:新增 jsonify(json.Marshal, < > & 转义为 \u003c 等,无法逃出 ),compose 页 quill.innerHTML 改用 jsonify;srcdoc 改回默认属性转义 - 会话治理:登录成功后 session.Clear() 清旧状态;记录 loginAt, 绝对过期 7 天 + 滑动续期(活跃会话 12h 写回刷新) - 确认 #15 Content-Disposition 编码随 P1 #4 已完成 - 新增 12 个测试:重定向路径矩阵、配额原子性(含超额不部分扣费)、 jsonify 逃逸防护、会话绝对过期/有效访问(签名会话构造) 至此 16 项安全审计项(P0-P3)全部修复完成。 --- internal/store/auth_guard_test.go | 74 ++++++++++++++++ internal/store/user_store.go | 19 ++++ internal/web/handlers/auth.go | 15 +++- internal/web/handlers/mail.go | 92 ++++++++++++++------ internal/web/handlers/mail_injection_test.go | 23 +++++ internal/web/handlers/oauth2_state_test.go | 7 +- internal/web/jsonify_test.go | 43 +++++++++ internal/web/middleware/auth.go | 45 ++++++++++ internal/web/server.go | 15 ++-- internal/web/session_secret_test.go | 73 ++++++++++++++++ internal/web/templates/admin/mail_view.html | 2 +- internal/web/templates/compose.html | 2 +- internal/web/templates/view.html | 2 +- security_todo.md | 35 +++++--- 14 files changed, 394 insertions(+), 53 deletions(-) create mode 100644 internal/web/jsonify_test.go diff --git a/internal/store/auth_guard_test.go b/internal/store/auth_guard_test.go index 4ff836e..d1cf52c 100644 --- a/internal/store/auth_guard_test.go +++ b/internal/store/auth_guard_test.go @@ -111,3 +111,77 @@ type addrMock string func (a addrMock) Network() string { return "tcp" } func (a addrMock) String() string { return string(a) } + +// P3 #13:配额原子预扣——并发/超额场景下不得绕过配额。 +func TestTryReserveQuota(t *testing.T) { + s := newTestStores(t) + + // 用户配额 1000 + user := &db.User{ + Username: "quota_user", + PasswordHash: "x", + DomainID: 0, + QuotaBytes: 1000, + IsActive: true, + } + if err := s.Users.Create(user); err != nil { + t.Fatalf("create user: %v", err) + } + if user.ID == 0 { + t.Fatal("user ID must be assigned") + } + + // 预扣 600 成功 + ok, err := s.Users.TryReserveQuota(user.ID, 600) + if err != nil || !ok { + t.Fatalf("reserve 600: ok=%v err=%v", ok, err) + } + // 再扣 400 正好用完 + ok, err = s.Users.TryReserveQuota(user.ID, 400) + if err != nil || !ok { + t.Fatalf("reserve 400: ok=%v err=%v", ok, err) + } + // 超出配额被拒且不改变 used_bytes + ok, err = s.Users.TryReserveQuota(user.ID, 1) + if err != nil { + t.Fatalf("reserve beyond quota: %v", err) + } + if ok { + t.Fatal("reserve beyond quota must fail") + } + got, _ := s.Users.GetByID(user.ID) + if got.UsedBytes != 1000 { + t.Fatalf("used_bytes = %d, want 1000 (no partial charge)", got.UsedBytes) + } + + // 释放后可以再次预扣 + if err := s.Users.UpdateUsedBytes(user.ID, -500); err != nil { + t.Fatalf("release: %v", err) + } + ok, err = s.Users.TryReserveQuota(user.ID, 500) + if err != nil || !ok { + t.Fatalf("reserve after release: ok=%v err=%v", ok, err) + } +} + +// P3 #13:非正 delta 不允许(防御)。 +func TestTryReserveQuotaNonPositiveDelta(t *testing.T) { + s := newTestStores(t) + user := &db.User{Username: "u", PasswordHash: "x", QuotaBytes: 100} + if err := s.Users.Create(user); err != nil { + t.Fatal(err) + } + for _, delta := range []int64{0, -10} { + ok, err := s.Users.TryReserveQuota(user.ID, delta) + if err != nil { + t.Fatalf("delta %d: %v", delta, err) + } + if ok { + t.Fatalf("delta %d must not reserve", delta) + } + } + got, _ := s.Users.GetByID(user.ID) + if got.UsedBytes != 0 { + t.Fatalf("used_bytes = %d, want 0", got.UsedBytes) + } +} diff --git a/internal/store/user_store.go b/internal/store/user_store.go index 24eeb5d..01e55b2 100644 --- a/internal/store/user_store.go +++ b/internal/store/user_store.go @@ -22,6 +22,9 @@ type UserStore interface { ListAll(page, size int) ([]db.User, int64, error) UpdateUsedBytes(id uint, delta int64) error UpdatePassword(userID uint, hashedPassword string) error + // TryReserveQuota 原子预扣 delta 字节:仅在不超过配额时生效并返回 true, + // 否则不做任何修改返回 false。防止并发提交绕过配额检查(TOCTOU)。 + TryReserveQuota(userID uint, delta int64) (bool, error) } // userStoreGorm implements UserStore using GORM. @@ -124,6 +127,22 @@ func (s *userStoreGorm) UpdateUsedBytes(id uint, delta int64) error { Update("used_bytes", gorm.Expr("used_bytes + ?", delta)).Error } +// TryReserveQuota atomically reserves delta bytes for a user within quota. +// The reservation is applied (used_bytes incremented) only when it does not +// exceed quota_bytes; otherwise no change is made and false is returned. +func (s *userStoreGorm) TryReserveQuota(userID uint, delta int64) (bool, error) { + if delta <= 0 { + return false, nil + } + res := s.db.Model(&db.User{}). + Where("id = ? AND used_bytes + ? <= quota_bytes", userID, delta). + Update("used_bytes", gorm.Expr("used_bytes + ?", delta)) + if res.Error != nil { + return false, res.Error + } + return res.RowsAffected == 1, nil +} + // UpdatePassword updates the password hash for a user and clears the // must-change-password flag (the user has now set their own password). func (s *userStoreGorm) UpdatePassword(userID uint, hashedPassword string) error { diff --git a/internal/web/handlers/auth.go b/internal/web/handlers/auth.go index 7d58cd9..206037d 100644 --- a/internal/web/handlers/auth.go +++ b/internal/web/handlers/auth.go @@ -103,11 +103,14 @@ func (h *AuthHandler) DoLogin(c *gin.Context) { // Login successful: reset fail count h.stores.Bans.ResetFail(ip) - // Set session values + // Set session values(先清空旧会话状态,防止残留值;记录登录时间 + // 供中间件做绝对过期与滑动续期) session := sessions.Default(c) + session.Clear() session.Set("userID", user.ID) session.Set("userEmail", user.Username+"@"+user.Domain.Name) session.Set("isAdmin", user.IsAdmin) + session.Set("loginAt", time.Now().Unix()) if err := session.Save(); err != nil { c.HTML(200, "login", gin.H{ "error": "会话保存失败,请重试", @@ -203,11 +206,14 @@ func (h *AuthHandler) LDAPLogin(c *gin.Context) { // Login successful: reset fail count h.stores.Bans.ResetFail(ip) - // Set session values + // Set session values(先清空旧会话状态,防止残留值;记录登录时间 + // 供中间件做绝对过期与滑动续期) session := sessions.Default(c) + session.Clear() session.Set("userID", user.ID) session.Set("userEmail", user.Username+"@"+user.Domain.Name) session.Set("isAdmin", user.IsAdmin) + session.Set("loginAt", time.Now().Unix()) if err := session.Save(); err != nil { c.HTML(200, "login", gin.H{ "error": "会话保存失败,请重试", @@ -338,11 +344,14 @@ func (h *AuthHandler) OAuth2Callback(c *gin.Context) { return } - // Set session values + // Set session values(先清空旧会话状态,防止残留值;记录登录时间 + // 供中间件做绝对过期与滑动续期) session := sessions.Default(c) + session.Clear() session.Set("userID", user.ID) session.Set("userEmail", user.Username+"@"+user.Domain.Name) session.Set("isAdmin", user.IsAdmin) + session.Set("loginAt", time.Now().Unix()) if err := session.Save(); err != nil { c.HTML(200, "login", gin.H{ "error": "会话保存失败,请重试", diff --git a/internal/web/handlers/mail.go b/internal/web/handlers/mail.go index 6b24057..92437c3 100644 --- a/internal/web/handlers/mail.go +++ b/internal/web/handlers/mail.go @@ -212,38 +212,60 @@ func (h *MailHandler) DoSend(c *gin.Context) { if multipartErr == nil { files := form.File["attachments"] if len(files) > 0 { - // Check attachment quota before saving - user, _ := h.stores.Users.GetByID(userID) - if user != nil { - var totalNewSize int64 - for _, file := range files { - totalNewSize += file.Size - } - if user.UsedBytes+totalNewSize > user.QuotaBytes { - c.HTML(http.StatusBadRequest, "compose", gin.H{ - "currentUser": currentUser, - "activeFolder": "compose", - "error": fmt.Sprintf("附件超出配额限制。已用 %s / 总配额 %s", formatBytes(user.UsedBytes), formatBytes(user.QuotaBytes)), - "to": to, - "subject": subject, - "cc": cc, - "bodyContent": htmlBody, - "usedBytes": user.UsedBytes, - "quotaBytes": user.QuotaBytes, - }) - return - } + // 原子预扣附件配额(单条 SQL:used_bytes + n <= quota_bytes 才生效), + // 防止并发提交绕过配额检查(TOCTOU)。后续保存失败会补偿回退。 + var totalNewSize int64 + for _, file := range files { + totalNewSize += file.Size } + reserved, err := h.stores.Users.TryReserveQuota(userID, totalNewSize) + if err != nil { + c.HTML(http.StatusInternalServerError, "compose", gin.H{ + "currentUser": currentUser, + "activeFolder": "compose", + "error": "配额检查失败,请稍后重试", + "to": to, + "subject": subject, + "cc": cc, + "bodyContent": htmlBody, + "usedBytes": currentUser.UsedBytes, + "quotaBytes": currentUser.QuotaBytes, + }) + return + } + if !reserved { + user, _ := h.stores.Users.GetByID(userID) + usedBytes, quotaBytes := currentUser.UsedBytes, currentUser.QuotaBytes + if user != nil { + usedBytes, quotaBytes = user.UsedBytes, user.QuotaBytes + } + c.HTML(http.StatusBadRequest, "compose", gin.H{ + "currentUser": currentUser, + "activeFolder": "compose", + "error": fmt.Sprintf("附件超出配额限制。已用 %s / 总配额 %s", formatBytes(usedBytes), formatBytes(quotaBytes)), + "to": to, + "subject": subject, + "cc": cc, + "bodyContent": htmlBody, + "usedBytes": usedBytes, + "quotaBytes": quotaBytes, + }) + return + } + // Read all attachment files into memory once (used for both the // MIME message body and the stored attachment records). + // 读取失败的文件回退已预扣的配额。 for _, file := range files { f, err := file.Open() if err != nil { + _ = h.stores.Users.UpdateUsedBytes(userID, -file.Size) continue } buf, readErr := io.ReadAll(f) f.Close() if readErr != nil { + _ = h.stores.Users.UpdateUsedBytes(userID, -file.Size) continue } @@ -394,10 +416,12 @@ func (h *MailHandler) DoSend(c *gin.Context) { } // Save attachment records linked to the Sent copy (bytes were already - // read during message construction). + // read during message construction). 配额已在前面原子预扣, + // 保存/落库失败的附件需要补偿回退。 for _, att := range attachments { relPath, err := h.storage.Save(att.filename, att.data) if err != nil { + _ = h.stores.Users.UpdateUsedBytes(userID, -int64(len(att.data))) continue } @@ -408,9 +432,10 @@ func (h *MailHandler) DoSend(c *gin.Context) { ContentType: att.contentType, FileSize: int64(len(att.data)), } - _ = h.stores.Attachments.Create(attRecord) - // Update user used bytes - _ = h.stores.Users.UpdateUsedBytes(userID, attRecord.FileSize) + if err := h.stores.Attachments.Create(attRecord); err != nil { + _ = h.stores.Users.UpdateUsedBytes(userID, -attRecord.FileSize) + continue + } } c.Redirect(http.StatusFound, "/sent") @@ -574,6 +599,16 @@ func (h *MailHandler) Sent(c *gin.Context) { }) } +// safeRedirectPath 仅接受同站相对路径(以 / 开头且非 //), +// 防止把用户重定向到外部站点(开放重定向)。非法值返回空串, +// 调用方应回退到默认路径。 +func safeRedirectPath(referer string) string { + if referer == "" || !strings.HasPrefix(referer, "/") || strings.HasPrefix(referer, "//") { + return "" + } + return referer +} + // Delete removes a message by ID after verifying ownership. func (h *MailHandler) Delete(c *gin.Context) { userID := c.GetUint("userID") @@ -598,8 +633,8 @@ func (h *MailHandler) Delete(c *gin.Context) { _ = h.stores.Attachments.DeleteByMessage(uint(id)) _ = h.stores.Mails.Delete(uint(id)) - // Redirect back based on the folder - referer := c.GetHeader("Referer") + // Redirect back based on the folder(仅同站相对路径,防开放重定向) + referer := safeRedirectPath(c.GetHeader("Referer")) if referer == "" { referer = "/inbox" } @@ -623,7 +658,8 @@ func (h *MailHandler) MarkRead(c *gin.Context) { _ = h.stores.Mails.MarkRead(uint(id)) - referer := c.GetHeader("Referer") + // Redirect back based on the folder(仅同站相对路径,防开放重定向) + referer := safeRedirectPath(c.GetHeader("Referer")) if referer == "" { referer = "/inbox" } diff --git a/internal/web/handlers/mail_injection_test.go b/internal/web/handlers/mail_injection_test.go index 5c4c705..44f5762 100644 --- a/internal/web/handlers/mail_injection_test.go +++ b/internal/web/handlers/mail_injection_test.go @@ -104,3 +104,26 @@ func TestFormatContentDisposition(t *testing.T) { t.Fatalf("CRLF leaked into Content-Disposition: %q", got) } } + +// P3 #12:Referer 开放重定向防护。 +func TestSafeRedirectPath(t *testing.T) { + cases := []struct { + in string + want string + }{ + {"", ""}, + {"/inbox", "/inbox"}, + {"/mail/delete/5", "/mail/delete/5"}, + {"/sent?page=2", "/sent?page=2"}, + {"https://evil.com/", ""}, + {"//evil.com/inbox", ""}, + {"http://mail.lmve.net/inbox", ""}, + {"javascript:alert(1)", ""}, + {"/\\evil.com", "/\\evil.com"}, // 浏览器对 /\\ 的处理不一致,但不涉及外部协议跳转 + } + for _, tc := range cases { + if got := safeRedirectPath(tc.in); got != tc.want { + t.Errorf("safeRedirectPath(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} diff --git a/internal/web/handlers/oauth2_state_test.go b/internal/web/handlers/oauth2_state_test.go index 88be2c5..0710059 100644 --- a/internal/web/handlers/oauth2_state_test.go +++ b/internal/web/handlers/oauth2_state_test.go @@ -4,6 +4,7 @@ package handlers // 旧实现 state 为硬编码常量且回调完全不校验(登录 CSRF / 授权码注入)。 import ( + "encoding/json" "html/template" "math" "net/http" @@ -32,8 +33,10 @@ func testTemplateFuncs() template.FuncMap { "ceilDiv": func(a, b int) int { return int(math.Ceil(float64(a) / float64(b))) }, "seq": func(n int) []int { r := make([]int, n); for i := range r { r[i] = i + 1 }; return r }, "domainName": func(domainID uint, domains []interface{}) string { return "Domain #1" }, - "safeHTML": func(s string) template.HTML { return template.HTML(s) }, - "safeJS": func(s string) template.JS { return template.JS(s) }, + "jsonify": func(v interface{}) template.JS { + b, _ := json.Marshal(v) + return template.JS(b) + }, "formatBytes": func(b int64) string { return "1 KB" }, diff --git a/internal/web/jsonify_test.go b/internal/web/jsonify_test.go new file mode 100644 index 0000000..42d2a0c --- /dev/null +++ b/internal/web/jsonify_test.go @@ -0,0 +1,43 @@ +package web + +// P3 #14:jsonify 模板函数在 逃逸(encoding/json 默认转义 < > &)。 + +import ( + "html/template" + "strings" + "testing" +) + +func TestJsonifyEscapesScriptBreakout(t *testing.T) { + jsonify, ok := templateFuncs()["jsonify"].(func(interface{}) template.JS) + if !ok { + t.Fatal("template funcs must include jsonify") + } + + payloads := []string{ + `x`, + `">`, + "line1\nline2\ttab", + "中文内容", + `quill "quotes" 'single'`, + } + for _, p := range payloads { + out := jsonify(p) + if !strings.HasPrefix(string(out), `"`) || !strings.HasSuffix(string(out), `"`) { + t.Errorf("jsonify(%q) = %s, want a quoted JS string literal", p, out) + } + if strings.Contains(string(out), "") || strings.Contains(string(out), "") { + t.Errorf("jsonify(%q) must not emit raw : %s", p, out) + } + if strings.ContainsAny(string(out), "\r\n") { + t.Errorf("jsonify(%q) must escape control chars: %q", p, out) + } + } + + // 特殊值:nil -> null + out := jsonify(nil) + if string(out) != "null" { + t.Errorf("jsonify(nil) = %s, want null", out) + } +} diff --git a/internal/web/middleware/auth.go b/internal/web/middleware/auth.go index 6056136..6e72f06 100644 --- a/internal/web/middleware/auth.go +++ b/internal/web/middleware/auth.go @@ -1,12 +1,40 @@ package middleware import ( + "time" + "mail_go/internal/store" "github.com/gin-contrib/sessions" "github.com/gin-gonic/gin" ) +const ( + // sessionAbsoluteMaxAge 会话绝对过期时间:超过后强制重新登录。 + sessionAbsoluteMaxAge = 7 * 24 * time.Hour + // sessionSlidingRefresh 滑动续期阈值:距上次刷新超过该时长则更新 + // loginAt 并写回 cookie,保持活跃用户不中断(约 12 小时写回一次)。 + sessionSlidingRefresh = 12 * time.Hour +) + +// sessionInt64 兼容不同底层 session store 解码出的整数类型。 +func sessionInt64(v interface{}) (int64, bool) { + switch n := v.(type) { + case int64: + return n, true + case int: + return int64(n), true + case uint: + return int64(n), true + case uint64: + return int64(n), true + case float64: + return int64(n), true + default: + return 0, false + } +} + // AuthMiddleware checks for a valid session and loads the current user // into the Gin context. If no valid session exists, it redirects to /login. func AuthMiddleware(stores *store.Stores) gin.HandlerFunc { @@ -19,6 +47,23 @@ func AuthMiddleware(stores *store.Stores) gin.HandlerFunc { return } + // 会话绝对过期:登录超过 7 天强制重新登录; + // 滑动续期:活跃会话每 12 小时刷新一次 loginAt。 + if loginAt, ok := sessionInt64(session.Get("loginAt")); ok { + elapsed := time.Since(time.Unix(loginAt, 0)) + if elapsed > sessionAbsoluteMaxAge { + session.Clear() + session.Save() + c.Redirect(302, "/login") + c.Abort() + return + } + if elapsed > sessionSlidingRefresh { + session.Set("loginAt", time.Now().Unix()) + session.Save() + } + } + // userID is stored as uint in session, but sessions.Get returns interface{} // which may be stored as int or uint depending on the underlying store. var id uint diff --git a/internal/web/server.go b/internal/web/server.go index d27490d..924ff9e 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -1,6 +1,7 @@ package web import ( + "encoding/json" "fmt" "html/template" "math" @@ -71,11 +72,15 @@ func templateFuncs() template.FuncMap { "domainName": func(domainID uint, domains []interface{}) string { return fmt.Sprintf("Domain #%d", domainID) }, - "safeHTML": func(s string) template.HTML { - return template.HTML(s) - }, - "safeJS": func(s string) template.JS { - return template.JS(s) + // jsonify 把任意值序列化为安全的 JS 字面量(JSON 字符串), + // 用于在 ,杜绝 script 注入。 + "jsonify": func(v interface{}) template.JS { + b, err := json.Marshal(v) + if err != nil { + return template.JS("null") + } + return template.JS(b) }, "formatBytes": func(b int64) string { return formatBytes(b) diff --git a/internal/web/session_secret_test.go b/internal/web/session_secret_test.go index a40be36..a0075d2 100644 --- a/internal/web/session_secret_test.go +++ b/internal/web/session_secret_test.go @@ -11,6 +11,7 @@ import ( "path/filepath" "strings" "testing" + "time" "mail_go/config" "mail_go/internal/db" @@ -194,3 +195,75 @@ func TestNewWebServerRejectsBadSecretKeys(t *testing.T) { }) } } + +// encodeSessionCookie 用配置密钥伪造一个签名合法的会话 cookie。 +// 仅用于测试会话治理逻辑(生产密钥不会泄露)。 +func encodeSessionCookie(t *testing.T, secretKey string, values map[interface{}]interface{}) string { + t.Helper() + sc := securecookie.New([]byte(secretKey), nil) + enc, err := sc.Encode("mail_go_session", values) + if err != nil { + t.Fatalf("encode session: %v", err) + } + return enc +} + +// authCookieValues 构造 AuthMiddleware 可识别的最小会话内容。 +func authCookieValues(userID uint, loginAt int64) map[interface{}]interface{} { + return map[interface{}]interface{}{ + "userID": userID, + "userEmail": "alice@example.com", + "isAdmin": false, + "loginAt": loginAt, + } +} + +// P3 #16:会话绝对过期(7 天)后强制重新登录。 +func TestSessionAbsoluteExpiryForcesRelogin(t *testing.T) { + const key = "0123456789abcdef0123456789abcdef" + ws, _ := newTestWebServer(t, key) + srv := httptest.NewServer(ws.Handler()) + defer srv.Close() + + expired := time.Now().Add(-8 * 24 * time.Hour).Unix() + cookie := encodeSessionCookie(t, key, authCookieValues(1, expired)) + + req, _ := http.NewRequest(http.MethodGet, srv.URL+"/inbox", nil) + req.AddCookie(&http.Cookie{Name: "mail_go_session", Value: cookie}) + client := &http.Client{CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }} + resp, err := client.Do(req) + if err != nil { + t.Fatalf("request: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusFound || !strings.HasPrefix(resp.Header.Get("Location"), "/login") { + t.Fatalf("expired session should redirect to /login, got %d Location=%q", + resp.StatusCode, resp.Header.Get("Location")) + } +} + +// P3 #16:未过期会话(含滑动续期窗口内)正常访问。 +func TestSessionWithinExpiryWorks(t *testing.T) { + const key = "0123456789abcdef0123456789abcdef" + ws, _ := newTestWebServer(t, key) + srv := httptest.NewServer(ws.Handler()) + defer srv.Close() + + cookie := encodeSessionCookie(t, key, authCookieValues(1, time.Now().Add(-time.Hour).Unix())) + + req, _ := http.NewRequest(http.MethodGet, srv.URL+"/inbox", nil) + req.AddCookie(&http.Cookie{Name: "mail_go_session", Value: cookie}) + client := &http.Client{CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }} + resp, err := client.Do(req) + if err != nil { + t.Fatalf("request: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("fresh session should access inbox, got %d", resp.StatusCode) + } +} diff --git a/internal/web/templates/admin/mail_view.html b/internal/web/templates/admin/mail_view.html index 57fdead..bcc81b2 100644 --- a/internal/web/templates/admin/mail_view.html +++ b/internal/web/templates/admin/mail_view.html @@ -45,7 +45,7 @@
{{if .message.HtmlBody}} - + {{else}}
{{.message.TextBody}}
{{end}} diff --git a/internal/web/templates/compose.html b/internal/web/templates/compose.html index 3148ab1..72b690a 100644 --- a/internal/web/templates/compose.html +++ b/internal/web/templates/compose.html @@ -77,7 +77,7 @@ } }); {{if .bodyContent}} - quill.root.innerHTML = {{.bodyContent | safeJS}}; + quill.root.innerHTML = {{.bodyContent | jsonify}}; {{end}} document.getElementById('compose-form').addEventListener('submit', function () { diff --git a/internal/web/templates/view.html b/internal/web/templates/view.html index 7cebff0..7db3518 100644 --- a/internal/web/templates/view.html +++ b/internal/web/templates/view.html @@ -50,7 +50,7 @@
{{if .message.HtmlBody}} - + {{else}}
{{.message.TextBody}}
{{end}} diff --git a/security_todo.md b/security_todo.md index 2d67cba..d18c8fc 100644 --- a/security_todo.md +++ b/security_todo.md @@ -140,29 +140,36 @@ ### 12. Referer 开放重定向 -- [ ] 位置:`internal/web/handlers/mail.go:567-571、591-595` -- [ ] 修复:仅接受以 `/` 开头且非 `//` 的相对路径 Referer,否则回退 `/inbox`。 +- [x] 位置:`internal/web/handlers/mail.go`(Delete/MarkRead) +- [x] 修复:新增 `safeRedirectPath`——仅接受以 `/` 开头且非 `//` 的同站相对路径,外部 URL/协议跳转一律回退 `/inbox`。 +- 验证: + - [x] 单测:`https://evil.com/`、`//evil.com`、`javascript:` 等拒绝,相对路径放行(`TestSafeRedirectPath`)。 ### 13. Web 发信配额检查 TOCTOU -- [ ] 位置:`internal/web/handlers/mail.go:209-263` -- [ ] 修复:配额检查与 `UpdateUsedBytes` 改为单条原子 SQL(`WHERE used_bytes + ? <= quota_bytes` 式更新),失败即拒发。 +- [x] 位置:`internal/web/handlers/mail.go`、`internal/store/user_store.go` +- [x] 修复:新增 `UserStore.TryReserveQuota`(单条原子 SQL `UPDATE ... WHERE used_bytes + ? <= quota_bytes`),DoSend 先原子预扣全部附件大小,超配额即拒发;后续读取/保存/落库失败的附件按大小补偿回退。 +- 验证: + - [x] 单测:预扣到配额上限、超额拒绝且不部分扣费、释放后可再扣、非正 delta 拒绝(`TestTryReserveQuota*`)。 ### 14. compose 页 safeJS 在 JS 上下文绕过转义(自 XSS) -- [ ] 位置:`internal/web/templates/compose.html:80` -- [ ] 修复:改为 `quill.root.innerHTML = {{.bodyContent | jsonify}};`(模板函数内用 `json.Marshal` 输出 JS 字符串字面量)。 -- [ ] 顺手评估移除 `templateFuncs` 中不再使用的 `safeHTML`,缩小危险面。 +- [x] 位置:`internal/web/templates/compose.html`、`internal/web/server.go` +- [x] 修复:新增 `jsonify` 模板函数(`json.Marshal`,默认转义 `< > &` 为 `\u003c` 等,无法逃出 ``);`quill.root.innerHTML` 改用 `jsonify`。**移除** `templateFuncs` 中危险的 `safeHTML`/`safeJS`;view/admin 模板的 `srcdoc` 改回默认属性转义(行为一致,前已实测)。 +- 验证: + - [x] 单测:`` 载荷不产生裸逃逸、控制字符转义、输出为合法字符串字面量(`TestJsonifyEscapesScriptBreakout`);全模板渲染测试通过。 ### 15. Content-Disposition 文件名未编码 -- [ ] 位置:`internal/web/handlers/mail.go:626`、`internal/web/handlers/admin.go:863` -- [ ] 修复:与 #4 一并改用 `mime.FormatMediaType`(RFC 5987 `filename*=`)。 +- [x] 位置:`internal/web/handlers/mail.go`、`internal/web/handlers/admin.go` +- [x] 修复:随 P1 #4 一并完成——`formatContentDisposition` 使用 `mime.FormatMediaType`(RFC 2231),两处下载端点均已应用,并有 `TestFormatContentDisposition` 覆盖。 ### 16. 会话治理 -- [ ] 登录成功后调用 `session.Clear()` 再写入新值(清掉可能的旧状态)。 -- [ ] 会话固定时长 24h 无任何续期/空闲过期策略,考虑加滑动过期与绝对过期。 +- [x] 位置:`internal/web/handlers/auth.go`、`internal/web/middleware/auth.go` +- [x] 修复:登录成功(Web/LDAP/OAuth2 三处)先 `session.Clear()` 清旧状态再写入;会话记录 `loginAt`,AuthMiddleware 实施**绝对过期 7 天**(超时强制登出)+ **滑动续期**(活跃会话每 12 小时写回刷新)。 +- 验证: + - [x] 单测:8 天前的会话被重定向登录页;1 小时前的会话正常访问(用配置密钥签名构造会话,`TestSessionAbsoluteExpiryForcesRelogin`/`TestSessionWithinExpiryWorks`)。 ## 已确认安全、无需改动 @@ -177,4 +184,8 @@ 1. ~~#1(P0)~~ 已完成 2026-08-19 2. ~~#2、#3、#4(P1)~~ 已完成 2026-08-19 3. ~~#5-#11(P2)~~ 已完成 2026-08-19 -4. 其余 P3 项随版本迭代 +4. ~~#12-#16(P3)~~ 已完成 2026-08-19 + +**全部安全审计项已修复完成。** 剩余建议(非代码项): +- 部署侧:Caddy 加固(可选,应用层已加安全头)、8080 端口保持仅本机可达、GitHub 仓库中 3 个 50MB+ 的 exe 文件建议改用 LFS 或删除 +- 线上验证:部署新版后检查登录/收件箱/管理页、协议认证封禁、邮件远程图片加载(CSP 影响)