From d832b1a41dcf42b1aa2ebbbde67c1b42f27fd52e Mon Sep 17 00:00:00 2001 From: Jannis Mattheis Date: Fri, 17 Jul 2026 14:09:48 +0200 Subject: [PATCH 1/2] fix: prevent csrf for cookie requests Prevously, the token was passed as X-Gotify-Key by the UI, so there was no csrf because no cookie was added by the browser to the request. The cookie is saved by SameSite=strict, this provides some protection against csrf. But an subdomain takeover could still allow for csrf. E.g. evil.gotify.net could send authenticated requests to gotify.net. This uses the go builtin cross origin protection, listed on the owasp page: https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#built-in-or-existing-csrf-implementations --- auth/authentication.go | 16 ++++++++++ auth/authentication_test.go | 63 ++++++++++++++++++++++++++++++++++++- router/router.go | 6 +++- 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/auth/authentication.go b/auth/authentication.go index 08ff8a0d7..6d8338f92 100644 --- a/auth/authentication.go +++ b/auth/authentication.go @@ -2,6 +2,7 @@ package auth import ( "errors" + "net/http" "strings" "time" @@ -40,6 +41,7 @@ type Database interface { type Auth struct { DB Database SecureCookie bool + CrossOrigin *http.CrossOriginProtection } // RequireAdmin requires an elevated client token or basic auth, the user must be an admin. @@ -87,6 +89,9 @@ func (a *Auth) Optional(ctx *gin.Context) { } func (a *Auth) evaluate(ctx *gin.Context, funcs ...func(ctx *gin.Context) (authState, error)) bool { + if a.rejectForeignOrigin(ctx) { + return true + } for _, fn := range funcs { state, err := fn(ctx) if err != nil { @@ -128,6 +133,17 @@ func (a *Auth) abort403(ctx *gin.Context) { ctx.AbortWithError(403, errors.New("you are not allowed to access this api")) } +func (a *Auth) rejectForeignOrigin(ctx *gin.Context) bool { + if _, isCookie := a.readTokenFromRequest(ctx); !isCookie { + return false + } + if err := a.CrossOrigin.Check(ctx.Request); err != nil { + ctx.AbortWithError(403, err) + return true + } + return false +} + func (a *Auth) handleUser(checks ...func(*model.User) (authState, error)) func(ctx *gin.Context) (authState, error) { return func(ctx *gin.Context) (authState, error) { if name, pass, ok := ctx.Request.BasicAuth(); ok { diff --git a/auth/authentication_test.go b/auth/authentication_test.go index f47fd9893..d92ecf10f 100644 --- a/auth/authentication_test.go +++ b/auth/authentication_test.go @@ -29,7 +29,7 @@ type AuthenticationSuite struct { func (s *AuthenticationSuite) SetupSuite() { mode.Set(mode.TestDev) s.DB = testdb.NewDB(s.T()) - s.auth = &Auth{DB: s.DB} + s.auth = &Auth{DB: s.DB, CrossOrigin: http.NewCrossOriginProtection()} now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) timeNow = func() time.Time { return now } @@ -354,4 +354,65 @@ func (s *AuthenticationSuite) assertHeaderRequest(key, value string, f fMiddlewa return ctx } +func (s *AuthenticationSuite) TestCookieCrossOriginProtection() { + // httptest sets the request host to example.com. + s.assertCsrfRequest(map[string]string{"Origin": "http://example.com"}, "clienttoken", s.auth.RequireClient, 200) + s.assertCsrfRequest(map[string]string{"Origin": "https://example.com"}, "clienttoken", s.auth.RequireClient, 200) + + s.assertCsrfRequest(map[string]string{"Origin": "http://evil.com"}, "clienttoken", s.auth.RequireClient, 403) + s.assertCsrfRequest(map[string]string{"Origin": "https://example.com.evil.com"}, "clienttoken", s.auth.RequireClient, 403) + s.assertCsrfRequest(map[string]string{"Origin": "null"}, "clienttoken", s.auth.RequireClient, 403) + + s.assertCsrfRequest(nil, "clienttoken", s.auth.RequireClient, 200) + + s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "same-origin"}, "clienttoken", s.auth.RequireClient, 200) + s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "none"}, "clienttoken", s.auth.RequireClient, 200) + s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "cross-site"}, "clienttoken", s.auth.RequireClient, 403) + s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "same-site"}, "clienttoken", s.auth.RequireClient, 403) + + s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "cross-site"}, "clienttoken_admin_elevated", s.auth.RequireElevatedClient, 403) + s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "same-origin"}, "clienttoken_admin_elevated", s.auth.RequireElevatedClient, 200) +} + +func (s *AuthenticationSuite) TestCrossOriginProtectionIgnoredForTokenAuth() { + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Request = httptest.NewRequest("POST", "/", nil) + ctx.Request.Header.Set("X-Gotify-Key", "clienttoken") + ctx.Request.Header.Set("Sec-Fetch-Site", "cross-site") + s.auth.RequireClient(ctx) + assert.Equal(s.T(), 200, recorder.Code) + + recorder = httptest.NewRecorder() + ctx, _ = gin.CreateTestContext(recorder) + ctx.Request = httptest.NewRequest("POST", "/?token=clienttoken", nil) + ctx.Request.Header.Set("Sec-Fetch-Site", "cross-site") + s.auth.RequireClient(ctx) + assert.Equal(s.T(), 200, recorder.Code) +} + +func (s *AuthenticationSuite) TestCrossOriginProtectionAllowsSafeMethods() { + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Request = httptest.NewRequest("GET", "/", nil) + ctx.Request.AddCookie(&http.Cookie{Name: cookieName, Value: "clienttoken"}) + ctx.Request.Header.Set("Sec-Fetch-Site", "cross-site") + s.auth.RequireClient(ctx) + assert.Equal(s.T(), 200, recorder.Code) +} + +func (s *AuthenticationSuite) assertCsrfRequest(headers map[string]string, cookie string, f fMiddleware, code int) { + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Request = httptest.NewRequest("POST", "/", nil) + if cookie != "" { + ctx.Request.AddCookie(&http.Cookie{Name: cookieName, Value: cookie}) + } + for k, v := range headers { + ctx.Request.Header.Set(k, v) + } + f(ctx) + assert.Equal(s.T(), code, recorder.Code) +} + type fMiddleware gin.HandlerFunc diff --git a/router/router.go b/router/router.go index a32933954..cf4d8da46 100644 --- a/router/router.go +++ b/router/router.go @@ -84,7 +84,11 @@ func Create(db *database.GormDatabase, vInfo *model.VersionInfo, conf *config.Co } } }() - authentication := auth.Auth{DB: db, SecureCookie: conf.Server.SecureCookie} + authentication := auth.Auth{ + DB: db, + SecureCookie: conf.Server.SecureCookie, + CrossOrigin: http.NewCrossOriginProtection(), + } messageHandler := api.MessageAPI{Notifier: streamHandler, DB: db} healthHandler := api.HealthAPI{DB: db} clientHandler := api.ClientAPI{ From 97c425c0abd5994a95f67636ec7be613531cb628 Mon Sep 17 00:00:00 2001 From: Jannis Mattheis Date: Fri, 17 Jul 2026 14:31:33 +0200 Subject: [PATCH 2/2] fix: appid mapping for sending messages --- model/message.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/model/message.go b/model/message.go index 6f75fe278..b18273954 100644 --- a/model/message.go +++ b/model/message.go @@ -32,7 +32,7 @@ type MessageExternal struct { // read only: true // required: true // example: 5 - ApplicationID uint `json:"appid"` + ApplicationID uint `form:"appid" query:"appid" json:"appid"` // The message. Markdown (excluding html) is allowed. // // required: true @@ -74,7 +74,7 @@ type CreateMessage struct { // The application id that send this message. Always set when returned via the API. // // example: 5 - ApplicationID uint `json:"appid"` + ApplicationID uint `form:"appid" query:"appid" json:"appid"` // The message. Markdown (excluding html) is allowed. // // required: true