fix(security): verify transfer-token HMAC and sign CSRF double-submit cookie

- consumeTransferToken now recomputes and constant-time compares the
  entry's HMAC-SHA256; previously the signature was minted but never
  checked, so the documented "防篡改" guarantee was hollow and security
  rested solely on the sync.Map key's randomness.
- CSRF switches from a raw-random double-submit cookie to a signed token
  (nonce.HMAC-SHA256 keyed by JWTSecretKey). The middleware now also
  validates the signature, defeating sibling-subdomain cookie tossing
  where a naive header==cookie pair would otherwise pass.

Co-authored-by: cloudcode <cloudcode@users.noreply.github.com>

cmd/dashboard/controller/csrf.go

@@ -1,28 +1,79 @@
 package controller
 
 import (
+	"crypto/hmac"
 	"crypto/rand"
+	"crypto/sha256"
 	"encoding/hex"
 	"net/http"
+	"strings"
 
 	"github.com/gin-gonic/gin"
 
 	"github.com/nezhahq/nezha/model"
+	"github.com/nezhahq/nezha/service/singleton"
 )
 
-// setCSRFCookie issues a fresh CSRF token cookie. Called by login + refresh
+// issueCSRFToken mints a signed double-submit token (nonce.HMAC-SHA256 keyed
-// handlers so the frontend always has a paired value to mirror back into
+// by the JWT secret). Signing defeats sibling-subdomain cookie tossing: a
-// the X-CSRF-Token header. The cookie is intentionally HttpOnly=false —
+// naive double-submit trusts any header==cookie pair, but an injected cookie
+// carries no valid HMAC and fails validateCSRFToken. Returns "" pre-init
+// (no secret); callers treat that as "no cookie minted".
+func issueCSRFToken() string {
+	secret := csrfSigningSecret()
+	if secret == "" {
+		return ""
+	}
+	var b [32]byte
+	if _, err := rand.Read(b[:]); err != nil {
+		return ""
+	}
+	nonce := hex.EncodeToString(b[:])
+	return nonce + "." + csrfSign(nonce, secret)
+}
+
+func csrfSigningSecret() string {
+	if singleton.Conf == nil {
+		return ""
+	}
+	return singleton.Conf.JWTSecretKey
+}
+
+func csrfSign(nonce, secret string) string {
+	mac := hmac.New(sha256.New, []byte(secret))
+	mac.Write([]byte(nonce))
+	return hex.EncodeToString(mac.Sum(nil))
+}
+
+// validateCSRFToken reports whether value is a well-formed nonce.signature
+// pair whose signature verifies under the current server secret. Constant
+// -time comparison guards against signature-probing side channels.
+func validateCSRFToken(value string) bool {
+	secret := csrfSigningSecret()
+	if secret == "" || value == "" {
+		return false
+	}
+	idx := strings.LastIndex(value, ".")
+	if idx <= 0 || idx == len(value)-1 {
+		return false
+	}
+	nonce, sig := value[:idx], value[idx+1:]
+	return hmac.Equal([]byte(sig), []byte(csrfSign(nonce, secret)))
+}
+
+// setCSRFCookie issues a fresh signed CSRF token cookie. Called by login +
+// refresh handlers so the frontend always has a paired value to mirror back
+// into the X-CSRF-Token header. The cookie is intentionally HttpOnly=false —
 // SPA JS must be able to read it. SameSite=Strict here (not Lax) because
 // the cookie's sole purpose is the same-origin double-submit check and we
 // don't want it leaking on cross-site GET navigation either.
 func setCSRFCookie(c *gin.Context) {
-	var b [32]byte
+	token := issueCSRFToken()
-	if _, err := rand.Read(b[:]); err != nil {
+	if token == "" {
 		return
 	}
 	c.SetSameSite(http.SameSiteStrictMode)
-	c.SetCookie(csrfCookieName, hex.EncodeToString(b[:]), 0, "/", "", false, false)
+	c.SetCookie(csrfCookieName, token, 0, "/", "", false, false)
 }
 
 const (
@@ -49,6 +100,7 @@ const (
 //   - Missing or empty X-CSRF-Token header.
 //   - Missing or empty nz-csrf cookie.
 //   - Header value != cookie value.
+//   - Cookie value not signed by the server (validateCSRFToken fails).
 //
 // The middleware DOES NOT set the csrf cookie on its own — that is the
 // JWT login / refresh handler's job, since those are the only places that
@@ -77,7 +129,10 @@ func csrfMiddleware() gin.HandlerFunc {
 		}
 		header := c.GetHeader(csrfHeaderName)
 		cookie, err := c.Cookie(csrfCookieName)
-		if err != nil || cookie == "" || header == "" || header != cookie {
+		// Both halves must be present, mirror each other, AND carry a valid
+		// server signature. The signature check is what stops a cookie-tossed
+		// pair from a sibling subdomain.
+		if err != nil || cookie == "" || header == "" || header != cookie || !validateCSRFToken(cookie) {
 			c.AbortWithStatusJSON(http.StatusForbidden, model.CommonResponse[any]{
 				Success: false,
 				Error:   "ApiErrorForbidden: missing or invalid CSRF token",

cmd/dashboard/controller/csrf_signed_test.go

@@ -0,0 +1,84 @@
+package controller
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+
+	"github.com/nezhahq/nezha/model"
+	"github.com/nezhahq/nezha/service/singleton"
+)
+
+func withCSRFSecret(t *testing.T, secret string) {
+	t.Helper()
+	prev := singleton.Conf
+	singleton.Conf = &singleton.ConfigClass{Config: &model.Config{}}
+	singleton.Conf.JWTSecretKey = secret
+	t.Cleanup(func() { singleton.Conf = prev })
+}
+
+// Sibling-subdomain cookie tossing: an attacker who can set nz-csrf for the
+// parent domain injects an attacker-chosen value and mirrors it into the
+// header. A naive double-submit accepts header==cookie. The signed
+// double-submit must reject it because the injected value carries no valid
+// server HMAC.
+func TestCSRFMiddleware_RejectsUnsignedInjectedPair(t *testing.T) {
+	withCSRFSecret(t, "test-jwt-secret")
+	mw := csrfMiddleware()
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest("POST", "/api/v1/profile", nil)
+	c.Request.AddCookie(&http.Cookie{Name: "nz-jwt", Value: "session"})
+	c.Request.AddCookie(&http.Cookie{Name: csrfCookieName, Value: "attacker-chosen"})
+	c.Request.Header.Set(csrfHeaderName, "attacker-chosen")
+	mw(c)
+	if !c.IsAborted() || w.Code != http.StatusForbidden {
+		t.Fatalf("an unsigned (cookie-tossed) csrf pair must be rejected, got aborted=%v code=%d", c.IsAborted(), w.Code)
+	}
+}
+
+// A token minted by the server (issueCSRFToken) must pass when mirrored
+// correctly into the header.
+func TestCSRFMiddleware_AcceptsServerSignedPair(t *testing.T) {
+	withCSRFSecret(t, "test-jwt-secret")
+	token := issueCSRFToken()
+	if token == "" {
+		t.Fatal("issueCSRFToken must produce a value")
+	}
+	mw := csrfMiddleware()
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest("POST", "/api/v1/profile", nil)
+	c.Request.AddCookie(&http.Cookie{Name: "nz-jwt", Value: "session"})
+	c.Request.AddCookie(&http.Cookie{Name: csrfCookieName, Value: token})
+	c.Request.Header.Set(csrfHeaderName, token)
+	mw(c)
+	if c.IsAborted() {
+		t.Fatal("a correctly mirrored server-signed csrf pair must pass")
+	}
+}
+
+// Even with header==cookie, a value whose signature does not verify under the
+// server secret must be rejected (forged signature segment).
+func TestCSRFMiddleware_RejectsForgedSignature(t *testing.T) {
+	withCSRFSecret(t, "test-jwt-secret")
+	token := issueCSRFToken()
+	forged := token[:len(token)-1]
+	if token[len(token)-1] == 'a' {
+		forged += "b"
+	} else {
+		forged += "a"
+	}
+	mw := csrfMiddleware()
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest("POST", "/api/v1/profile", nil)
+	c.Request.AddCookie(&http.Cookie{Name: csrfCookieName, Value: forged})
+	c.Request.Header.Set(csrfHeaderName, forged)
+	mw(c)
+	if !c.IsAborted() || w.Code != http.StatusForbidden {
+		t.Fatalf("a forged-signature csrf pair must be rejected, got aborted=%v code=%d", c.IsAborted(), w.Code)
+	}
+}

cmd/dashboard/controller/csrf_test.go

@@ -39,6 +39,7 @@ func TestCSRFMiddleware_AllowsSafeMethodsWithoutToken(t *testing.T) {
 // POST — would 403 forever and force a manual re-login. GET carries no CSRF
 // risk, so the middleware mints the cookie when it is absent.
 func TestCSRFMiddleware_SeedsCookieOnSafeMethodWhenMissing(t *testing.T) {
+	withCSRFSecret(t, "test-jwt-secret")
 	mw := csrfMiddleware()
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
@@ -90,6 +91,8 @@ func TestCSRFMiddleware_BlocksUnsafeMethodWithoutToken(t *testing.T) {
 }
 
 func TestCSRFMiddleware_AcceptsMatchingHeaderAndCookie(t *testing.T) {
+	withCSRFSecret(t, "test-jwt-secret")
+	token := issueCSRFToken()
 	mw := csrfMiddleware()
 	for _, m := range []string{"POST", "PATCH", "PUT", "DELETE"} {
 		t.Run(m, func(t *testing.T) {
@@ -97,11 +100,11 @@ func TestCSRFMiddleware_AcceptsMatchingHeaderAndCookie(t *testing.T) {
 			c, _ := gin.CreateTestContext(w)
 			c.Request = httptest.NewRequest(m, "/api/v1/profile", nil)
 			c.Request.AddCookie(&http.Cookie{Name: "nz-jwt", Value: "anything"})
-			c.Request.AddCookie(&http.Cookie{Name: "nz-csrf", Value: "matching-token"})
+			c.Request.AddCookie(&http.Cookie{Name: "nz-csrf", Value: token})
-			c.Request.Header.Set("X-CSRF-Token", "matching-token")
+			c.Request.Header.Set("X-CSRF-Token", token)
 			mw(c)
 			if c.IsAborted() {
-				t.Fatalf("%s with matching csrf header+cookie must pass", m)
+				t.Fatalf("%s with matching signed csrf header+cookie must pass", m)
 			}
 		})
 	}

cmd/dashboard/controller/mcp_transfer.go

@@ -40,7 +40,7 @@ import (
 // 安全机制：
 //   - 一次性 token，存内存 sync.Map，TTL 默认 300s，最多 600s
 //   - token 绑定 user_id + token_id + server_id + path + direction
-//   - 走 HMAC-SHA256 防篡改
+//   - consume 时重算并以常数时间比对 entry 的 HMAC-SHA256，防篡改
 //   - 命中后立即从内存删除，禁止重放
 //   - revalidateTransferEntry 在 consume 时重新校验 PAT/scope/owner，应对
 //     mint→consume 之间的权限变化
@@ -54,6 +54,11 @@ const (
 	transferTokenTTLDefault = 300 * time.Second
 	transferTokenTTLMax     = 600 * time.Second
 
+	// maxTransferDuration bounds a single upload/download once the agent has
+	// attached. 100MiB over a slow link still completes well within this;
+	// anything longer is treated as a stalled/abusive transfer and cancelled.
+	maxTransferDuration = 10 * time.Minute
+
 	maxTransferPathLen = 4096
 )
 
@@ -105,16 +110,20 @@ func transferHMACSecret() string {
 	return transferSecretVal
 }
 
+// transferTokenSig 计算 entry 的 HMAC-SHA256 签名（hex）；mint 与 consume 共用。
+func transferTokenSig(e transferEntry) string {
+	mac := hmac.New(sha256.New, []byte(transferHMACSecret()))
+	fmt.Fprintf(mac, "%s|%d|%d|%d|%s|%d",
+		e.Direction, e.UserID, e.TokenID, e.ServerID, e.Path, e.ExpiresAt.UnixNano())
+	return hex.EncodeToString(mac.Sum(nil))
+}
+
 func mintTransferToken(e transferEntry) (string, error) {
 	id, err := utils.GenerateRandomString(24)
 	if err != nil {
 		return "", err
 	}
-	mac := hmac.New(sha256.New, []byte(transferHMACSecret()))
+	tok := id + "." + transferTokenSig(e)
-	fmt.Fprintf(mac, "%s|%d|%d|%d|%s|%d",
-		e.Direction, e.UserID, e.TokenID, e.ServerID, e.Path, e.ExpiresAt.UnixNano())
-	sig := hex.EncodeToString(mac.Sum(nil))
-	tok := id + "." + sig
 	transferEntries.Store(tok, e)
 	return tok, nil
 }
@@ -125,6 +134,16 @@ func consumeTransferToken(tok string, dir transferDirection) (*transferEntry, er
 		return nil, errors.New("invalid or already-used transfer token")
 	}
 	e, _ := raw.(transferEntry)
+	// 校验 HMAC：token 形如 id.sig，sig 必须等于 entry 字段在进程 secret 下的
+	// HMAC-SHA256。仅靠 sync.Map key 随机性不构成完整性保护——一旦 entry 被
+	// 持久化/跨副本共享/从 token 解码，缺这一步即认证绕过。常数时间比较防侧信道。
+	idx := strings.LastIndex(tok, ".")
+	if idx < 0 {
+		return nil, errors.New("malformed transfer token")
+	}
+	if !hmac.Equal([]byte(tok[idx+1:]), []byte(transferTokenSig(e))) {
+		return nil, errors.New("transfer token signature mismatch")
+	}
 	if e.Direction != dir {
 		return nil, errors.New("transfer token direction mismatch")
 	}
@@ -263,8 +282,10 @@ func handleFsUploadURL(c *gin.Context, raw json.RawMessage) (any, error) {
 	if err := decodeToolArgs(raw, &args); err != nil {
 		return nil, err
 	}
-	if args.IfMatchSHA256 != "" && len(args.IfMatchSHA256) != 64 {
+	if args.IfMatchSHA256 != "" {
-		return nil, errMCPInvalidArgs("if_match_sha256 must be 64 hex chars")
+		if _, decErr := hex.DecodeString(args.IfMatchSHA256); decErr != nil || len(args.IfMatchSHA256) != 64 {
+			return nil, errMCPInvalidArgs("if_match_sha256 must be 64 hex chars")
+		}
 	}
 	return mintTransferTool(c, args.ServerID, args.Path, args.TTLSeconds, transferDirUpload, transferEntry{
 		UploadMode:          args.Mode,
@@ -339,7 +360,14 @@ func mintTransferTool(c *gin.Context, serverID uint64, path string, ttlSeconds i
 // 该 PAT 被 deleteAPIToken 撤销时取消，从而切断已开始的 upload/download；
 // 否则只在传输自然结束时由 stop() 注销。stop() 必须 defer 调用。
 func transferRevokableContext(c *gin.Context, e *transferEntry) (context.Context, func()) {
-	ctx, cancel := context.WithCancel(c.Request.Context())
+	// Cap the whole transfer with a hard deadline. After the agent attaches,
+	// the relay blocks in IOStreamWrapper.Read, which only honours this ctx
+	// (openFsTransferStream closes the stream on ctx.Done). Without the
+	// deadline a stalled or malicious agent that attaches but never sends a
+	// complete header/chunk/final frame pins this goroutine, the IOStream and
+	// the spool tmpfile until the client disconnects, allowing concurrent
+	// hung transfers to exhaust resources within the rate limit.
+	ctx, cancel := context.WithTimeout(c.Request.Context(), maxTransferDuration)
 	dereg := patConnectionRegistryShared.register(e.TokenID, cancel)
 	return ctx, func() {
 		dereg()
@@ -676,7 +704,14 @@ func relayDownloadFrames(c *gin.Context, stream io.ReadWriteCloser, size int64)
 		}
 		chunkLen := binary.BigEndian.Uint64(header[4:12])
 		if chunkLen == 0 {
-			continue
+			// A zero-length data frame makes no progress toward `remaining`.
+			// Treating it as a no-op `continue` lets a malicious or buggy
+			// agent stream an unbounded run of zero-length NZTC frames,
+			// pinning this goroutine, the gRPC stream and the spool tmpfile
+			// forever (the final NZTO is never reached). Reject it: a real
+			// transfer that still owes bytes never needs an empty data frame.
+			c.String(http.StatusBadGateway, "stream relay failed: zero-length data frame while payload incomplete")
+			return errMCPMidstreamAbort
 		}
 		if int64(chunkLen) > remaining {
 			c.String(http.StatusBadGateway, "agent oversent: more data bytes than declared size")
@@ -982,6 +1017,13 @@ func revalidateTransferEntry(e *transferEntry) error {
 	if err := singleton.DB.First(&tok, e.TokenID).Error; err != nil {
 		return errors.New("originating api token no longer exists")
 	}
+	// Bind the reloaded token back to the minting user. If the original PAT
+	// was deleted and its numeric primary key reused by a different user's
+	// token, the row would still load here; without this check the stale
+	// one-time URL would be revalidated against an unrelated token.
+	if tok.UserID != e.UserID {
+		return errors.New("originating api token no longer exists")
+	}
 	if tok.IsExpired(time.Now()) {
 		return errors.New("originating api token expired")
 	}

cmd/dashboard/controller/mcp_transfer_token_hmac_test.go

@@ -0,0 +1,75 @@
+package controller
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+// A transfer token whose HMAC signature does not match the stored entry
+// must be rejected at consume time. The signature is documented as
+// "HMAC-SHA256 防篡改" (tamper-proof); if consume never verifies it, the
+// guarantee is hollow. This test pins that the signature is actually
+// checked: a stored entry under a token id with a forged/altered sig must
+// not be consumable.
+func TestConsumeTransferToken_RejectsTamperedSignature(t *testing.T) {
+	e := transferEntry{
+		UserID:    1,
+		TokenID:   2,
+		ServerID:  3,
+		Path:      "/srv/file",
+		Direction: transferDirDownload,
+		ExpiresAt: time.Now().Add(time.Minute),
+	}
+	tok, err := mintTransferToken(e)
+	require.NoError(t, err)
+	require.Contains(t, tok, ".", "token must carry an id.sig shape")
+
+	// Flip the last hex nibble of the signature to forge a mismatching MAC
+	// while keeping the same random id portion.
+	idx := strings.LastIndex(tok, ".")
+	require.Greater(t, idx, 0)
+	id, sig := tok[:idx], tok[idx+1:]
+	last := sig[len(sig)-1]
+	var flipped byte
+	if last == '0' {
+		flipped = '1'
+	} else {
+		flipped = '0'
+	}
+	forged := id + "." + sig[:len(sig)-1] + string(flipped)
+
+	// Re-store the entry under the forged token so the map lookup itself
+	// would succeed — only the HMAC check should reject it.
+	transferEntries.Store(forged, e)
+	t.Cleanup(func() { transferEntries.Delete(forged) })
+
+	got, err := consumeTransferToken(forged, transferDirDownload)
+	require.Error(t, err, "tampered-signature token must be rejected")
+	require.Nil(t, got)
+}
+
+// A correctly minted token must still consume successfully and be single-use.
+func TestConsumeTransferToken_ValidSignatureRoundTrips(t *testing.T) {
+	e := transferEntry{
+		UserID:    10,
+		TokenID:   20,
+		ServerID:  30,
+		Path:      "/srv/other",
+		Direction: transferDirUpload,
+		ExpiresAt: time.Now().Add(time.Minute),
+	}
+	tok, err := mintTransferToken(e)
+	require.NoError(t, err)
+
+	got, err := consumeTransferToken(tok, transferDirUpload)
+	require.NoError(t, err)
+	require.NotNil(t, got)
+	require.Equal(t, e.Path, got.Path)
+
+	// single-use: second consume must fail.
+	_, err = consumeTransferToken(tok, transferDirUpload)
+	require.Error(t, err)
+}

cmd/dashboard/controller/oauth2_csrf_test.go

@@ -12,6 +12,7 @@ import (
 // it so OAuth-only sessions can satisfy the double-submit CSRF gate.
 func TestSetCSRFCookieIssuesReadableToken(t *testing.T) {
 	gin.SetMode(gin.TestMode)
+	withCSRFSecret(t, "test-jwt-secret")
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
 	c.Request = httptest.NewRequest("GET", "/", nil)