diff --git a/server/channels/app/plugin_requests.go b/server/channels/app/plugin_requests.go index b9d6bdde2ae..41a972e22d1 100644 --- a/server/channels/app/plugin_requests.go +++ b/server/channels/app/plugin_requests.go @@ -5,6 +5,7 @@ package app import ( "bytes" + "errors" "io" "net/http" "path" @@ -267,7 +268,18 @@ func (ch *Channels) servePluginRequest(w http.ResponseWriter, r *http.Request, h return } - if validateCSRFForPluginRequest(rctx, r, session, cookieAuth, *ch.cfgSvc.Config().ServiceSettings.ExperimentalStrictCSRFEnforcement) { + maxBodyBytes := *ch.cfgSvc.Config().ServiceSettings.MaximumPayloadSizeBytes + bytes.MinRead + csrfValid, err := validateCSRFForPluginRequest(rctx, w, r, session, cookieAuth, *ch.cfgSvc.Config().ServiceSettings.ExperimentalStrictCSRFEnforcement, maxBodyBytes) + if err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + http.Error(w, http.StatusText(http.StatusRequestEntityTooLarge), http.StatusRequestEntityTooLarge) + return + } + handleInternalServerError(rctx, "Failed to read request body for plugin CSRF validation", err) + return + } + if csrfValid { r.Header.Set("Mattermost-User-Id", session.UserId) context.SessionId = session.Id } else { @@ -278,18 +290,19 @@ func (ch *Channels) servePluginRequest(w http.ResponseWriter, r *http.Request, h } // validateCSRFForPluginRequest validates CSRF token for plugin requests -func validateCSRFForPluginRequest(rctx request.CTX, r *http.Request, session *model.Session, cookieAuth bool, strictCSRFEnforcement bool) bool { +func validateCSRFForPluginRequest(rctx request.CTX, w http.ResponseWriter, r *http.Request, session *model.Session, cookieAuth bool, strictCSRFEnforcement bool, maxBodyBytes int64) (bool, error) { // Skip CSRF check for non-cookie auth or GET requests if !cookieAuth || r.Method == http.MethodGet { - return true + return true, nil } csrfTokenFromClient := r.Header.Get(model.HeaderCsrfToken) if csrfTokenFromClient == "" { + r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes) bodyBytes, err := io.ReadAll(r.Body) if err != nil { - rctx.Logger().Warn("Failed to read request body for plugin request", mlog.Err(err)) + return false, err } r.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) if err := r.ParseForm(); err != nil { @@ -301,7 +314,7 @@ func validateCSRFForPluginRequest(rctx request.CTX, r *http.Request, session *mo expectedToken := session.GetCSRF() if csrfTokenFromClient == expectedToken { - return true + return true, nil } // ToDo(DSchalla) 2019/01/04: Remove after deprecation period and only allow CSRF Header (MM-13657) @@ -309,13 +322,13 @@ func validateCSRFForPluginRequest(rctx request.CTX, r *http.Request, session *mo csrfErrorMessage := "CSRF Check failed for request - Please migrate your plugin to either send a CSRF Header or Form Field, XMLHttpRequest is deprecated" if strictCSRFEnforcement { rctx.Logger().Warn(csrfErrorMessage, mlog.String("session_id", session.Id)) - return false + return false, nil } // Allow XMLHttpRequest for backward compatibility when not strict rctx.Logger().Debug(csrfErrorMessage, mlog.String("session_id", session.Id)) - return true + return true, nil } - return false + return false, nil } diff --git a/server/channels/app/plugin_requests_test.go b/server/channels/app/plugin_requests_test.go index 0af2b9293b0..6c863d92bd4 100644 --- a/server/channels/app/plugin_requests_test.go +++ b/server/channels/app/plugin_requests_test.go @@ -570,6 +570,31 @@ func TestServePluginRequest(t *testing.T) { require.True(t, handlerCalled) }) + t.Run("oversized cookie auth POST without CSRF header returns 413 and is not forwarded", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.MaximumPayloadSizeBytes = 1 }) + t.Cleanup(func() { + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.MaximumPayloadSizeBytes = 300000 }) + }) + + body := strings.NewReader(strings.Repeat("a", 2048)) + req := httptest.NewRequest(http.MethodPost, "/plugins/testplugin/endpoint", body) + req = mux.SetURLVars(req, map[string]string{"plugin_id": "testplugin"}) + req.AddCookie(&http.Cookie{ + Name: model.SessionCookieToken, + Value: session.Token, + }) + rr := httptest.NewRecorder() + + handlerCalled := false + mockHandler := func(ctx *plugin.Context, w http.ResponseWriter, r *http.Request) { + handlerCalled = true + } + + th.App.ch.servePluginRequest(rr, req, mockHandler) + assert.Equal(t, http.StatusRequestEntityTooLarge, rr.Code) + assert.False(t, handlerCalled) + }) + t.Run("third-party use of Authorization header preserved", func(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/plugins/testplugin/endpoint", nil) req.Header.Set(model.HeaderAuth, "Bearer 3rd-party-token") @@ -590,12 +615,14 @@ func TestServePluginRequest(t *testing.T) { func TestValidateCSRFForPluginRequest(t *testing.T) { th := Setup(t) + const testMaxBodyBytes = 1024 * 1024 + t.Run("skip CSRF for non-cookie auth", func(t *testing.T) { session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"} session.GenerateCSRF() req := httptest.NewRequest(http.MethodPost, "/test", nil) - result := validateCSRFForPluginRequest(th.Context, req, session, false, false) + result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, false, false, testMaxBodyBytes) assert.True(t, result) }) @@ -604,7 +631,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) { session.GenerateCSRF() req := httptest.NewRequest(http.MethodGet, "/test", nil) - result := validateCSRFForPluginRequest(th.Context, req, session, true, false) + result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes) assert.True(t, result) }) @@ -614,7 +641,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/test", nil) req.Header.Set(model.HeaderCsrfToken, expectedToken) - result := validateCSRFForPluginRequest(th.Context, req, session, true, false) + result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes) assert.True(t, result) }) @@ -624,7 +651,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/test", nil) req.Header.Set(model.HeaderCsrfToken, "invalid-token") - result := validateCSRFForPluginRequest(th.Context, req, session, true, false) + result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes) assert.False(t, result) }) @@ -635,10 +662,60 @@ func TestValidateCSRFForPluginRequest(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(formData)) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - result := validateCSRFForPluginRequest(th.Context, req, session, true, false) + result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes) assert.True(t, result) }) + t.Run("oversized form body is rejected as too large", func(t *testing.T) { + const smallMaxBodyBytes = 128 + + session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"} + expectedToken := session.GenerateCSRF() + formData := "filler=" + strings.Repeat("a", smallMaxBodyBytes) + "&csrf=" + expectedToken + req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(formData)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + valid, err := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, smallMaxBodyBytes) + assert.False(t, valid) + var maxBytesErr *http.MaxBytesError + assert.ErrorAs(t, err, &maxBytesErr) + }) + + t.Run("oversized form body with early csrf token is rejected, not accepted with truncated data", func(t *testing.T) { + const smallMaxBodyBytes = 64 + + session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"} + expectedToken := session.GenerateCSRF() + // A valid token up front must not let an over-limit body through with truncated data. + formData := "csrf=" + expectedToken + "&filler=" + strings.Repeat("a", smallMaxBodyBytes) + req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(formData)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + valid, err := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, smallMaxBodyBytes) + assert.False(t, valid) + var maxBytesErr *http.MaxBytesError + assert.ErrorAs(t, err, &maxBytesErr) + }) + + t.Run("oversized body with valid csrf header is not read or capped", func(t *testing.T) { + const smallMaxBodyBytes = 64 + + session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"} + expectedToken := session.GenerateCSRF() + body := strings.Repeat("a", smallMaxBodyBytes*4) + req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(body)) + req.Header.Set(model.HeaderCsrfToken, expectedToken) + + valid, err := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, smallMaxBodyBytes) + assert.True(t, valid) + require.NoError(t, err) + + // The header path must leave the body untouched for the plugin to stream. + read, err := io.ReadAll(req.Body) + require.NoError(t, err) + assert.Len(t, read, smallMaxBodyBytes*4) + }) + t.Run("XMLHttpRequest with strict enforcement disabled", func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ExperimentalStrictCSRFEnforcement = false @@ -649,7 +726,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/test", nil) req.Header.Set(model.HeaderRequestedWith, model.HeaderRequestedWithXML) - result := validateCSRFForPluginRequest(th.Context, req, session, true, false) + result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes) assert.True(t, result) }) @@ -663,7 +740,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/test", nil) req.Header.Set(model.HeaderRequestedWith, model.HeaderRequestedWithXML) - result := validateCSRFForPluginRequest(th.Context, req, session, true, true) + result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, true, testMaxBodyBytes) assert.False(t, result) }) }