Forráskód Böngészése

fix(security): require a 2FA code to replace the stored TOTP secret

The confirmation gate in updateSetting only covered the true -> false
transition, so a settings save that kept twoFactorEnable=true while
carrying a non-blank twoFactorToken silently rebound the authenticator.
preserveRedactedSecrets restores the stored secret only when the
submitted one is blank, so a non-blank value went straight through
without any branch asking for a code.

Not reachable pre-auth or cross-site (CSRFMiddleware rejects unsafe
methods without the session token), but it matters after a session
hijack or with an admin API token, which sets api_authed and
short-circuits the CSRF check: the attacker gains persistence and locks
the legitimate operator out of their own authenticator.

Now a code is required whenever 2FA is currently on and the submitted
secret differs from the stored one. Enabling from off is untouched, as
no code exists yet to verify, and a blank secret still means
"unchanged", so the panel's normal save path is unaffected.

Reported by @n0ctal (GHSA-xqqw-jqqv-99h6).
Sanaei 7 órája
szülő
commit
c8a3a2d723

+ 11 - 4
internal/web/controller/setting.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"net/http"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/mhsanaei/3x-ui/v3/internal/logger"
@@ -130,10 +131,16 @@ func (a *SettingController) updateSetting(c *gin.Context) {
 	oldTgToken, _ := a.settingService.GetTgBotToken()
 	oldTgChatId, _ := a.settingService.GetTgBotChatId()
 	oldTgAPIServer, _ := a.settingService.GetTgBotAPIServer()
-	if twoFactorErr == nil && oldTwoFactor && !allSetting.TwoFactorEnable {
-		if err := a.settingService.VerifyTwoFactorCode(form.TwoFactorCode); err != nil {
-			jsonMsg(c, I18nWeb(c, "pages.settings.toasts.modifySettings"), err)
-			return
+	if twoFactorErr == nil && oldTwoFactor {
+		// Rebinding the authenticator is the same class of change as turning 2FA
+		// off, so both need a current code. Blank still means "unchanged".
+		submittedToken := strings.TrimSpace(allSetting.TwoFactorToken)
+		storedToken, _ := a.settingService.GetTwoFactorToken()
+		if !allSetting.TwoFactorEnable || (submittedToken != "" && submittedToken != storedToken) {
+			if err := a.settingService.VerifyTwoFactorCode(form.TwoFactorCode); err != nil {
+				jsonMsg(c, I18nWeb(c, "pages.settings.toasts.modifySettings"), err)
+				return
+			}
 		}
 	}
 	err := a.settingService.UpdateAllSetting(allSetting, service.SecretClears{

+ 84 - 0
internal/web/controller/setting_test.go

@@ -1,6 +1,7 @@
 package controller
 
 import (
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"path/filepath"
@@ -13,6 +14,7 @@ import (
 	"github.com/mhsanaei/3x-ui/v3/internal/database"
 	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
 	"github.com/mhsanaei/3x-ui/v3/internal/util/crypto"
+	"github.com/mhsanaei/3x-ui/v3/internal/web/service"
 )
 
 func TestValidateRegex(t *testing.T) {
@@ -86,3 +88,85 @@ func TestAPITokenMutationRoutesEnforceExpectedScope(t *testing.T) {
 		t.Fatal("token was disabled by wrong scope")
 	}
 }
+
+// GHSA-xqqw-jqqv-99h6: a save that keeps 2FA enabled must not be able to
+// rebind the authenticator without presenting a current code.
+func TestUpdateSettingRequiresCodeToReplaceTwoFactorToken(t *testing.T) {
+	t.Setenv("XUI_DB_FOLDER", t.TempDir())
+	if err := database.InitDB(filepath.Join(t.TempDir(), "x-ui.db")); err != nil {
+		t.Fatalf("InitDB: %v", err)
+	}
+	t.Cleanup(func() { _ = database.CloseDB() })
+
+	settingService := service.SettingService{}
+	if err := settingService.SetTwoFactorToken("ORIGINALSECRET234567"); err != nil {
+		t.Fatalf("seed token: %v", err)
+	}
+	if err := settingService.SetTwoFactorEnable(true); err != nil {
+		t.Fatalf("seed enable: %v", err)
+	}
+
+	post := func(t *testing.T, mutate func(map[string]any)) string {
+		t.Helper()
+		base, err := settingService.GetAllSetting()
+		if err != nil {
+			t.Fatalf("GetAllSetting: %v", err)
+		}
+		raw, err := json.Marshal(base)
+		if err != nil {
+			t.Fatalf("marshal: %v", err)
+		}
+		body := map[string]any{}
+		if err := json.Unmarshal(raw, &body); err != nil {
+			t.Fatalf("unmarshal: %v", err)
+		}
+		mutate(body)
+		payload, err := json.Marshal(body)
+		if err != nil {
+			t.Fatalf("marshal payload: %v", err)
+		}
+
+		gin.SetMode(gin.TestMode)
+		router := gin.New()
+		NewSettingController(router.Group("/panel/api"))
+		req := httptest.NewRequest(http.MethodPost, "/panel/api/setting/update", strings.NewReader(string(payload)))
+		req.Header.Set("Content-Type", "application/json")
+		resp := httptest.NewRecorder()
+		router.ServeHTTP(resp, req)
+		return resp.Body.String()
+	}
+
+	t.Run("rebind without code is rejected", func(t *testing.T) {
+		got := post(t, func(body map[string]any) {
+			body["twoFactorEnable"] = true
+			body["twoFactorToken"] = "ATTACKERSECRET567890"
+		})
+		if !strings.Contains(got, `"success":false`) {
+			t.Fatalf("rebind without a 2FA code was accepted: %s", got)
+		}
+		stored, err := settingService.GetTwoFactorToken()
+		if err != nil {
+			t.Fatalf("GetTwoFactorToken: %v", err)
+		}
+		if stored != "ORIGINALSECRET234567" {
+			t.Fatalf("stored 2FA secret = %q, want it unchanged", stored)
+		}
+	})
+
+	t.Run("ordinary save with redacted token still succeeds", func(t *testing.T) {
+		got := post(t, func(body map[string]any) {
+			body["twoFactorEnable"] = true
+			body["twoFactorToken"] = ""
+		})
+		if !strings.Contains(got, `"success":true`) {
+			t.Fatalf("normal settings save was rejected: %s", got)
+		}
+		stored, err := settingService.GetTwoFactorToken()
+		if err != nil {
+			t.Fatalf("GetTwoFactorToken: %v", err)
+		}
+		if stored != "ORIGINALSECRET234567" {
+			t.Fatalf("stored 2FA secret = %q, want it preserved", stored)
+		}
+	})
+}