From 4dfb3facb4b8144825950a83f10f6134f2de983c Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 21 Aug 2025 00:39:06 +0200 Subject: [PATCH 1/2] fix: validate CSRF on non-safe methods - CSRF has to be validated for any request that can change the state, in practice this means any HTTP request where the method isn't GET/HEAD/OPTIONS. - The code only considered POST to be a state-changing request. - Forgejo has several PUT/DELETE (that changes state) routes for which no CSRF was being validated. - Change the code to validate CSRF for all non-"safe" methods. --- routers/web/web.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routers/web/web.go b/routers/web/web.go index a47ce2bff7..43ce0dba6d 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -192,7 +192,8 @@ func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.Cont return } - if !options.SignOutRequired && !options.DisableCSRF && ctx.Req.Method == "POST" { + safeMethod := ctx.Req.Method == "GET" || ctx.Req.Method == "HEAD" || ctx.Req.Method == "OPTIONS" + if !options.SignOutRequired && !options.DisableCSRF && !safeMethod { ctx.Csrf.Validate(ctx) if ctx.Written() { return From 5fdd6ce9a6b67089884297ffdd7b49e4463f0fcb Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 21 Aug 2025 01:04:29 +0200 Subject: [PATCH 2/2] chore: add integration test Verify that PUT/DELETE requests return invalid CSRF token when no CSRF token is given with the request. --- tests/integration/csrf_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/integration/csrf_test.go b/tests/integration/csrf_test.go index 100614cbb4..f548faeda6 100644 --- a/tests/integration/csrf_test.go +++ b/tests/integration/csrf_test.go @@ -1,4 +1,5 @@ // Copyright 2017 The Gitea Authors. All rights reserved. +// Copyright 2025 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package integration @@ -32,3 +33,23 @@ func TestCsrfProtection(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusBadRequest) assert.Contains(t, resp.Body.String(), "Invalid CSRF token") } + +func TestCSRFSafeMethods(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + t.Run("DELETE", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + resp := session.MakeRequest(t, NewRequest(t, "DELETE", "/user2/repo1/projects/1/2"), http.StatusBadRequest) + assert.Equal(t, "Invalid CSRF token.\n", resp.Body.String()) + }) + + t.Run("PUT", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + resp := session.MakeRequest(t, NewRequest(t, "PUT", "/user2/repo1/projects/1/2"), http.StatusBadRequest) + assert.Equal(t, "Invalid CSRF token.\n", resp.Body.String()) + }) +}