From 254733b0566830a46e5a44c2d3127f57bfaceb65 Mon Sep 17 00:00:00 2001 From: grm Date: Fri, 18 Sep 2026 13:46:33 +0300 Subject: Security: Cap the login body and log failed logins POST /webadmin is the one form outside guardPOST, so nothing bounded its body: a multipart login could park 32 MB in memory or temp files per request. It now reads at most 64 KB. Wrong passwords are logged with the username and client address so an attack shows up in the log (fail2ban can read it) instead of being invisible. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Sd8UPWrvyYCLj97JexNw3A --- AGENTS.md | 5 ++++- internal/web/handlers_auth.go | 11 +++++++++++ internal/web/web_test.go | 13 +++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 577ab82..406c621 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -135,7 +135,10 @@ internal/web/ server.go (host router, middleware, render helpers) - **Hardening** (`web/ratelimit.go`): a per-key token bucket throttles the anonymous endpoints worth abusing — `loginLimit` on `POST /webadmin`, keyed by client address *and* by lowercased username (10 at once, then 10 a - minute each), answered with 429 by `s.throttle` and a log line. Flood + minute each), answered with 429 by `s.throttle` and a log line; failed + logins are logged with the username and address, and the login body is + capped at `maxLoginBody` (64 KB) since it is the one POST outside + `guardPOST`. Flood control for everything else stays at the reverse proxy (`limit_req`). - **Templates**: each page file is parsed together with its layout (`layouts/dashboard.html` or `layouts/blog.html` for `blog/*`) and all diff --git a/internal/web/handlers_auth.go b/internal/web/handlers_auth.go index 1545a4e..69b7b98 100644 --- a/internal/web/handlers_auth.go +++ b/internal/web/handlers_auth.go @@ -44,7 +44,16 @@ func (s *Server) handleWebadminRedirect(w http.ResponseWriter, r *http.Request) http.Redirect(w, r, s.cfg.RootURL()+"/webadmin?next="+urlQuery("/b/"+sub+"/"), http.StatusSeeOther) } +// maxLoginBody is all a login form needs; it is the one POST guardPOST does +// not cap, so it caps itself. +const maxLoginBody = 64 << 10 + func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) { + r.Body = http.MaxBytesReader(w, r.Body, maxLoginBody) + if err := r.ParseForm(); err != nil { + s.plainError(w, http.StatusBadRequest, s.tr(r, "Could not read the form.")) + return + } username := strings.TrimSpace(r.FormValue("username")) password := r.FormValue("password") next := safeNext(r.FormValue("next")) @@ -64,10 +73,12 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) { return } auth.CheckPassword("$2a$10$invalidinvalidinvalidinvalidinvalidinvalidinvalidinval", password) // constant-ish time + logf("login failed for %q from %s", username, clientIP(r)) fail() return } if u.Disabled || !auth.CheckPassword(u.PasswordHash, password) { + logf("login failed for %q from %s", username, clientIP(r)) fail() return } diff --git a/internal/web/web_test.go b/internal/web/web_test.go index 6324443..49915f4 100644 --- a/internal/web/web_test.go +++ b/internal/web/web_test.go @@ -962,3 +962,16 @@ func TestLoginThrottled(t *testing.T) { t.Errorf("got %d, want 429", rec.Code) } } + +// The login form is the one POST outside guardPOST; it caps its own body. +func TestLoginBodyCapped(t *testing.T) { + s := NewServer(&config.Config{BaseDomain: "example.com", JWTSecret: []byte("x")}, nil) + rec := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/webadmin", strings.NewReader("username=a&password="+strings.Repeat("x", maxLoginBody))) + req.Host = "example.com" + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + s.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("got %d, want 400", rec.Code) + } +} -- cgit v1.2.3