From 2e14a2a51a275246abb63b9a4faffadb6fa72714 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sat, 31 Oct 2026 09:04:27 +0000 Subject: [PATCH] fix(web): refuse oversized login usernames before recording them A failed login keeps its username in the in-memory log buffer, the limiter table and the notifier queue, and the route accepted any body the global cap allowed. The login route now takes at most 16 KiB and answers a username over 256 bytes with the generic failure before any of those see it; the user setters refuse such a name too, so no stored username can be one the login route turns away. --- internal/web/controller/csrf_log_test.go | 53 ++++++++++++++++++++++++ internal/web/controller/index.go | 9 +++- internal/web/service/panel/user.go | 10 +++++ internal/web/service/panel/user_test.go | 26 ++++++++++++ 4 files changed, 97 insertions(+), 1 deletion(-) diff --git a/internal/web/controller/csrf_log_test.go b/internal/web/controller/csrf_log_test.go index e436bcf39..90b3d2fdb 100644 --- a/internal/web/controller/csrf_log_test.go +++ b/internal/web/controller/csrf_log_test.go @@ -1,8 +1,11 @@ package controller import ( + "encoding/json" "net/http" "net/http/httptest" + "net/url" + "path/filepath" "strings" "testing" @@ -10,7 +13,11 @@ import ( "github.com/gin-contrib/sessions/cookie" "github.com/gin-gonic/gin" + "github.com/mhsanaei/3x-ui/v3/internal/database/dbtest" "github.com/mhsanaei/3x-ui/v3/internal/logger" + "github.com/mhsanaei/3x-ui/v3/internal/web/locale" + "github.com/mhsanaei/3x-ui/v3/internal/web/service/panel" + "github.com/mhsanaei/3x-ui/v3/internal/web/session" ) func TestLoginCSRFRejectionLogsPeerIPNotForwardedHeader(t *testing.T) { @@ -39,3 +46,49 @@ func TestLoginCSRFRejectionLogsPeerIPNotForwardedHeader(t *testing.T) { t.Fatalf("CSRF rejection log = %q, want the untrusted peer 198.51.100.9 and not the forwarded header", line) } } + +// A failed login keeps its username in the log buffer, the limiter table and the +// notifier queue; an unauthenticated caller must not be able to make that unbounded. +func TestLoginRefusesOversizedUsernameBeforeRecordingIt(t *testing.T) { + gin.SetMode(gin.TestMode) + dbtest.InitDB(t, filepath.Join(t.TempDir(), "x-ui.db")) + engine := gin.New() + engine.Use(sessions.Sessions("3x-ui", cookie.NewStore([]byte("login-length-test-secret")))) + engine.Use(func(c *gin.Context) { + c.Set("I18n", func(_ locale.I18nType, key string, _ ...string) string { return key }) + c.Next() + }) + NewIndexController(engine.Group("/")) + + tokenReq := httptest.NewRequest(http.MethodGet, "/csrf-token", nil) + tokenRec := httptest.NewRecorder() + engine.ServeHTTP(tokenRec, tokenReq) + var token struct{ Obj string } + if err := json.Unmarshal(tokenRec.Body.Bytes(), &token); err != nil || token.Obj == "" { + t.Fatalf("csrf token: %v %s", err, tokenRec.Body.String()) + } + username := strings.Repeat("u", panel.MaxUsernameLength+1) + form := url.Values{"username": {username}, "password": {"wrong"}} + req := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(form.Encode())) + req.RemoteAddr = "198.51.100.10:40000" + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set(session.CSRFHeaderName, token.Obj) + for _, cookie := range tokenRec.Result().Cookies() { + req.AddCookie(cookie) + } + rec := httptest.NewRecorder() + engine.ServeHTTP(rec, req) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"success":false`) { + t.Fatalf("login = %d %s, want the generic failure", rec.Code, rec.Body.String()) + } + for _, line := range logger.GetLogs(100, "DEBUG") { + if strings.Contains(line, username) { + t.Fatal("the oversized username reached the log buffer") + } + } + defaultLoginLimiter.mu.Lock() + defer defaultLoginLimiter.mu.Unlock() + if _, kept := defaultLoginLimiter.attempts[loginLimitKey("198.51.100.10", username)]; kept { + t.Fatal("the oversized username was kept in the limiter table") + } +} diff --git a/internal/web/controller/index.go b/internal/web/controller/index.go index 0433e2ed8..ba0bae364 100644 --- a/internal/web/controller/index.go +++ b/internal/web/controller/index.go @@ -47,7 +47,8 @@ func (a *IndexController) initRouter(g *gin.RouterGroup) { g.GET("/sponsors", a.sponsors) g.GET("/sponsors/logo/:name", a.sponsorLogo) - g.POST("/login", middleware.CSRFMiddleware(getRemoteIp), a.login) + // The route carries three short fields; anything longer is refused unread. + g.POST("/login", middleware.MaxBodyBytes(16<<10), middleware.CSRFMiddleware(getRemoteIp), a.login) g.POST("/logout", middleware.CSRFMiddleware(getRemoteIp), a.logout) g.POST("/getTwoFactorEnable", middleware.CSRFMiddleware(getRemoteIp), a.getTwoFactorEnable) } @@ -102,6 +103,12 @@ func (a *IndexController) login(c *gin.Context) { } remoteIP := getRemoteIp(c) + // No stored name is longer; the log buffer, the limiter and the notifiers keep what follows. + if len(form.Username) > panel.MaxUsernameLength { + logger.Warningf("failed login: IP=%q, reason=%q", remoteIP, "username too long") + pureJsonMsg(c, http.StatusOK, false, I18nWeb(c, "pages.login.toasts.wrongUsernameOrPassword")) + return + } safeUser := template.HTMLEscapeString(form.Username) timeStr := time.Now().Format("2006-01-02 15:04:05") if blockedUntil, ok := defaultLoginLimiter.allow(remoteIP, form.Username); !ok { diff --git a/internal/web/service/panel/user.go b/internal/web/service/panel/user.go index dd6970d86..8c0bf9e7d 100644 --- a/internal/web/service/panel/user.go +++ b/internal/web/service/panel/user.go @@ -2,6 +2,7 @@ package panel import ( "errors" + "fmt" "time" "gorm.io/gorm" @@ -128,6 +129,9 @@ func (s *UserService) BumpLoginEpoch() error { } func (s *UserService) UpdateUser(id int, username string, password string) error { + if len(username) > MaxUsernameLength { + return fmt.Errorf("username can not be longer than %d bytes", MaxUsernameLength) + } db := database.GetDB() hashedPassword, err := crypto.HashPasswordAsBcrypt(password) if err != nil { @@ -157,6 +161,8 @@ func (s *UserService) UpdateUser(id int, username string, password string) error func (s *UserService) UpdateFirstUser(username string, password string) error { if username == "" { return errors.New("username can not be empty") + } else if len(username) > MaxUsernameLength { + return fmt.Errorf("username can not be longer than %d bytes", MaxUsernameLength) } else if password == "" { return errors.New("password can not be empty") } @@ -181,3 +187,7 @@ func (s *UserService) UpdateFirstUser(username string, password string) error { user.LoginEpoch++ return db.Save(user).Error } + +// MaxUsernameLength bounds a panel username: the login route refuses longer names before +// logging or recording them, so no stored name may exceed it. +const MaxUsernameLength = 256 diff --git a/internal/web/service/panel/user_test.go b/internal/web/service/panel/user_test.go index ba79578d4..70e26f5b3 100644 --- a/internal/web/service/panel/user_test.go +++ b/internal/web/service/panel/user_test.go @@ -2,6 +2,7 @@ package panel import ( "path/filepath" + "strings" "testing" "github.com/mhsanaei/3x-ui/v3/internal/database" @@ -48,3 +49,28 @@ func TestHasDefaultCredential(t *testing.T) { }) } } + +// The login route refuses names over MaxUsernameLength unread, so storing one would lock +// the admin out of their own panel. +func TestUserSettersRefuseNamesTheLoginRouteRefuses(t *testing.T) { + dbtest.InitDB(t, filepath.Join(t.TempDir(), "x-ui.db")) + svc := &UserService{} + if err := svc.UpdateFirstUser("admin", "s3cret-Pass"); err != nil { + t.Fatalf("UpdateFirstUser: %v", err) + } + var user model.User + if err := database.GetDB().First(&user).Error; err != nil { + t.Fatalf("load user: %v", err) + } + long := strings.Repeat("a", MaxUsernameLength+1) + if err := svc.UpdateFirstUser(long, "s3cret-Pass"); err == nil { + t.Fatal("UpdateFirstUser stored a name the login route refuses") + } + if err := svc.UpdateUser(user.Id, long, "s3cret-Pass"); err == nil { + t.Fatal("UpdateUser stored a name the login route refuses") + } + var stored model.User + if err := database.GetDB().First(&stored, user.Id).Error; err != nil || stored.Username != "admin" { + t.Fatalf("stored username = %q (%v), want it unchanged", stored.Username, err) + } +}