From 92b5f3a9e1f91098bbc25acd37bec7873172a6fd Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 01:40:05 +0100 Subject: [PATCH] Bound security request decoding --- internal/api/bootstrap_token.go | 5 +- internal/api/router.go | 16 +-- internal/api/router_routes_auth_security.go | 4 +- internal/api/security_request_body.go | 46 +++++++ internal/api/security_request_body_test.go | 143 ++++++++++++++++++++ internal/api/security_setup_fix.go | 8 +- 6 files changed, 205 insertions(+), 17 deletions(-) create mode 100644 internal/api/security_request_body.go create mode 100644 internal/api/security_request_body_test.go diff --git a/internal/api/bootstrap_token.go b/internal/api/bootstrap_token.go index 6d5fc71a2..3d4d05efb 100644 --- a/internal/api/bootstrap_token.go +++ b/internal/api/bootstrap_token.go @@ -3,7 +3,6 @@ package api import ( "crypto/rand" "encoding/hex" - "encoding/json" "errors" "net/http" "os" @@ -146,8 +145,8 @@ func (r *Router) handleValidateBootstrapToken(w http.ResponseWriter, req *http.R var payload struct { Token string `json:"token"` } - if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { - http.Error(w, "Invalid request payload", http.StatusBadRequest) + if err := decodeSecurityRequestBody(w, req, &payload); err != nil { + http.Error(w, "Invalid request payload", securityRequestErrorStatus(err)) return } token = strings.TrimSpace(payload.Token) diff --git a/internal/api/router.go b/internal/api/router.go index fb9cc015a..9231c6f04 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -3904,8 +3904,8 @@ func (r *Router) handleUpdateRelayConfig(w http.ResponseWriter, req *http.Reques InstanceSecret *string `json:"instance_secret"` AlertMinimumSeverity *string `json:"alert_minimum_severity"` } - if err := json.NewDecoder(req.Body).Decode(&update); err != nil { - http.Error(w, "invalid request body", http.StatusBadRequest) + if err := decodeSecurityRequestBody(w, req, &update); err != nil { + http.Error(w, "invalid request body", securityRequestErrorStatus(err)) return } @@ -5326,8 +5326,8 @@ func (r *Router) handleChangePassword(w http.ResponseWriter, req *http.Request) NewPassword string `json:"newPassword"` } - if err := json.NewDecoder(req.Body).Decode(&changeReq); err != nil { - writeErrorResponse(w, http.StatusBadRequest, "invalid_request", + if err := decodeSecurityRequestBody(w, req, &changeReq); err != nil { + writeErrorResponse(w, securityRequestErrorStatus(err), "invalid_request", "Invalid request body", nil) return } @@ -5696,8 +5696,8 @@ func (r *Router) handleLogin(w http.ResponseWriter, req *http.Request) { RememberMe bool `json:"rememberMe"` } - if err := json.NewDecoder(req.Body).Decode(&loginReq); err != nil { - writeErrorResponse(w, http.StatusBadRequest, "invalid_request", + if err := decodeSecurityRequestBody(w, req, &loginReq); err != nil { + writeErrorResponse(w, securityRequestErrorStatus(err), "invalid_request", "Invalid request body", nil) return } @@ -5877,8 +5877,8 @@ func (r *Router) handleResetLockout(w http.ResponseWriter, req *http.Request) { Identifier string `json:"identifier"` // Can be username or IP } - if err := json.NewDecoder(req.Body).Decode(&resetReq); err != nil { - writeErrorResponse(w, http.StatusBadRequest, "invalid_request", + if err := decodeSecurityRequestBody(w, req, &resetReq); err != nil { + writeErrorResponse(w, securityRequestErrorStatus(err), "invalid_request", "Invalid request body", nil) return } diff --git a/internal/api/router_routes_auth_security.go b/internal/api/router_routes_auth_security.go index 14f433891..60b3194c2 100644 --- a/internal/api/router_routes_auth_security.go +++ b/internal/api/router_routes_auth_security.go @@ -544,8 +544,8 @@ func (r *Router) registerAuthSecurityInstallRoutes() { Duration int `json:"duration,omitempty"` // Duration in minutes for token generation } - if err := json.NewDecoder(req.Body).Decode(&recoveryRequest); err != nil { - http.Error(w, "Invalid request", http.StatusBadRequest) + if err := decodeSecurityRequestBody(w, req, &recoveryRequest); err != nil { + http.Error(w, "Invalid request", securityRequestErrorStatus(err)) return } diff --git a/internal/api/security_request_body.go b/internal/api/security_request_body.go new file mode 100644 index 000000000..5de16fcc9 --- /dev/null +++ b/internal/api/security_request_body.go @@ -0,0 +1,46 @@ +package api + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "net/http" +) + +// Security requests carry only credentials and small control fields. Keeping a +// shared bound prevents public authentication and recovery routes from +// allocating attacker-controlled JSON strings without limit. +const maxSecurityRequestBodyBytes int64 = 16 * 1024 + +func decodeSecurityRequestBody(w http.ResponseWriter, r *http.Request, dst any) error { + if r == nil || r.Body == nil { + return io.EOF + } + + r.Body = http.MaxBytesReader(w, r.Body, maxSecurityRequestBodyBytes) + decoder := json.NewDecoder(r.Body) + if err := decoder.Decode(dst); err != nil { + return err + } + + // A second decode both rejects concatenated JSON documents and forces the + // bounded reader to consume trailing whitespace. Without it, a valid first + // object could hide an arbitrarily large unread request tail. + var trailing any + if err := decoder.Decode(&trailing); err != io.EOF { + if err == nil { + return fmt.Errorf("request body must contain a single JSON value") + } + return err + } + return nil +} + +func securityRequestErrorStatus(err error) int { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + return http.StatusRequestEntityTooLarge + } + return http.StatusBadRequest +} diff --git a/internal/api/security_request_body_test.go b/internal/api/security_request_body_test.go new file mode 100644 index 000000000..ba141ba00 --- /dev/null +++ b/internal/api/security_request_body_test.go @@ -0,0 +1,143 @@ +package api + +import ( + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +func TestDecodeSecurityRequestBodyEnforcesWholeDocumentLimit(t *testing.T) { + prefix := `{"value":"` + suffix := `"}` + exact := prefix + strings.Repeat("a", int(maxSecurityRequestBodyBytes)-len(prefix)-len(suffix)) + suffix + + var decoded struct { + Value string `json:"value"` + } + req := httptest.NewRequest(http.MethodPost, "/api/login", strings.NewReader(exact)) + if err := decodeSecurityRequestBody(httptest.NewRecorder(), req, &decoded); err != nil { + t.Fatalf("exact-limit body rejected: %v", err) + } + + req = httptest.NewRequest(http.MethodPost, "/api/login", strings.NewReader(exact+" ")) + err := decodeSecurityRequestBody(httptest.NewRecorder(), req, &decoded) + var maxBytesErr *http.MaxBytesError + if !errors.As(err, &maxBytesErr) { + t.Fatalf("over-limit trailing input error = %v, want *http.MaxBytesError", err) + } +} + +func TestLoginRejectsOversizedAndConcatenatedJSON(t *testing.T) { + router := newLoginRouter(t) + + tests := []struct { + name string + body string + wantStatus int + }{ + { + name: "oversized trailing input", + body: `{"username":"admin","password":"wrong"}` + strings.Repeat(" ", int(maxSecurityRequestBodyBytes)), + wantStatus: http.StatusRequestEntityTooLarge, + }, + { + name: "concatenated objects", + body: `{"username":"admin","password":"wrong"}{"username":"admin","password":"Password!1"}`, + wantStatus: http.StatusBadRequest, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/api/login", strings.NewReader(tt.body)) + req.RemoteAddr = "192.0.2.80:1234" + rec := httptest.NewRecorder() + + router.handleLogin(rec, req) + + if rec.Code != tt.wantStatus { + t.Fatalf("status = %d, want %d (%s)", rec.Code, tt.wantStatus, rec.Body.String()) + } + }) + } +} + +func TestPublicBootstrapHandlersRejectOversizedJSONBeforeCredentialUse(t *testing.T) { + t.Run("validation", func(t *testing.T) { + router := &Router{ + bootstrapTokenHash: "configured", + bootstrapTokenValidationLimiter: NewRateLimiter(10, 5*time.Minute), + } + t.Cleanup(router.bootstrapTokenValidationLimiter.Stop) + + body := `{"token":"` + strings.Repeat("a", int(maxSecurityRequestBodyBytes)) + `"}` + req := httptest.NewRequest(http.MethodPost, "/api/security/validate-bootstrap-token", strings.NewReader(body)) + req.RemoteAddr = "192.0.2.81:1234" + rec := httptest.NewRecorder() + + router.handleValidateBootstrapToken(rec, req) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want %d (%s)", rec.Code, http.StatusRequestEntityTooLarge, rec.Body.String()) + } + }) + + t.Run("quick setup", func(t *testing.T) { + cfg := &config.Config{DataPath: t.TempDir(), ConfigPath: t.TempDir()} + router := &Router{config: cfg} + clientIP := "192.0.2.82" + authLimiter.Reset(clientIP) + t.Cleanup(func() { authLimiter.Reset(clientIP) }) + + body := `{"username":"` + strings.Repeat("a", int(maxSecurityRequestBodyBytes)) + `"}` + req := httptest.NewRequest(http.MethodPost, "/api/security/quick-setup", strings.NewReader(body)) + req.RemoteAddr = clientIP + ":1234" + rec := httptest.NewRecorder() + + handleQuickSecuritySetupFixed(router)(rec, req) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want %d (%s)", rec.Code, http.StatusRequestEntityTooLarge, rec.Body.String()) + } + if cfg.AuthUser != "" || cfg.AuthPass != "" || cfg.HasAPITokens() { + t.Fatalf("oversized request changed auth state: user=%q pass_set=%v tokens=%d", cfg.AuthUser, cfg.AuthPass != "", len(cfg.APITokens)) + } + }) + + t.Run("recovery", func(t *testing.T) { + router := &Router{mux: http.NewServeMux(), config: &config.Config{}} + router.registerAuthSecurityInstallRoutes() + + body := `{"action":"` + strings.Repeat("a", int(maxSecurityRequestBodyBytes)) + `"}` + req := httptest.NewRequest(http.MethodPost, "/api/security/recovery", strings.NewReader(body)) + req.RemoteAddr = "127.0.0.1:1234" + rec := httptest.NewRecorder() + + router.mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want %d (%s)", rec.Code, http.StatusRequestEntityTooLarge, rec.Body.String()) + } + }) +} + +func FuzzDecodeSecurityRequestBody(f *testing.F) { + f.Add(`{"username":"admin","password":"Password!1"}`) + f.Add(`{"username":"admin"}{"password":"Password!1"}`) + f.Add("{") + f.Add("") + + f.Fuzz(func(t *testing.T, body string) { + var decoded map[string]any + req := httptest.NewRequest(http.MethodPost, "/api/login", strings.NewReader(body)) + err := decodeSecurityRequestBody(httptest.NewRecorder(), req, &decoded) + if err == nil && int64(len(body)) > maxSecurityRequestBodyBytes { + t.Fatalf("accepted %d-byte body above %d-byte limit", len(body), maxSecurityRequestBodyBytes) + } + }) +} diff --git a/internal/api/security_setup_fix.go b/internal/api/security_setup_fix.go index 065e1c300..56f461ac2 100644 --- a/internal/api/security_setup_fix.go +++ b/internal/api/security_setup_fix.go @@ -298,8 +298,8 @@ func handleQuickSecuritySetupFixed(r *Router) http.HandlerFunc { SetupToken string `json:"setupToken"` } - if err := json.NewDecoder(req.Body).Decode(&setupRequest); err != nil { - http.Error(w, "Invalid request body", http.StatusBadRequest) + if err := decodeSecurityRequestBody(w, req, &setupRequest); err != nil { + http.Error(w, "Invalid request body", securityRequestErrorStatus(err)) return } @@ -789,8 +789,8 @@ func (r *Router) HandleValidateAPIToken(w http.ResponseWriter, rq *http.Request) Token string `json:"token"` } - if err := json.NewDecoder(rq.Body).Decode(&validateRequest); err != nil { - http.Error(w, "Invalid request body", http.StatusBadRequest) + if err := decodeSecurityRequestBody(w, rq, &validateRequest); err != nil { + http.Error(w, "Invalid request body", securityRequestErrorStatus(err)) return }