From 422044317ccd530d1f392039fe432c5512a1603e Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 7 Jul 2026 13:07:15 +0100 Subject: [PATCH] security: GHSA-xpxj-f2fm-rqch (HIGH) bound OAuth2 state map growth Sweep expired OAuth2 state entries, cap the map at 10000 entries, and remove stale state on failed callback validation. Co-authored-by: Cursor --- .../auth/otoauth2/restapi_auth_oauth2.go | 32 +++++++++ .../auth/otoauth2/restapi_auth_oauth2_test.go | 66 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 service/internal/auth/otoauth2/restapi_auth_oauth2_test.go diff --git a/service/internal/auth/otoauth2/restapi_auth_oauth2.go b/service/internal/auth/otoauth2/restapi_auth_oauth2.go index 351a13b..c0a1c02 100644 --- a/service/internal/auth/otoauth2/restapi_auth_oauth2.go +++ b/service/internal/auth/otoauth2/restapi_auth_oauth2.go @@ -62,8 +62,14 @@ type oauth2State struct { providerName string Username string Usergroup string + createdAt time.Time } +const ( + oauthStateMaxAge = 900 // matches olivetin-sid-oauth cookie MaxAge + oauthStateMaxEntries = 10000 +) + func assignIfEmpty(target *string, value string) { if *target == "" { *target = value @@ -129,6 +135,19 @@ func (h *OAuth2Handler) setOAuthCallbackCookie(w http.ResponseWriter, r *http.Re http.SetCookie(w, cookie) } +func (h *OAuth2Handler) deleteOAuthStateLocked(state string) { + delete(h.registeredStates, state) +} + +func (h *OAuth2Handler) sweepExpiredOAuthStatesLocked(now time.Time) { + cutoff := now.Add(-oauthStateMaxAge * time.Second) + for state, entry := range h.registeredStates { + if entry.createdAt.Before(cutoff) { + delete(h.registeredStates, state) + } + } +} + func (h *OAuth2Handler) HandleOAuthLogin(w http.ResponseWriter, r *http.Request) { state, err := randString(16) @@ -147,10 +166,17 @@ func (h *OAuth2Handler) HandleOAuthLogin(w http.ResponseWriter, r *http.Request) } h.mu.Lock() + h.sweepExpiredOAuthStatesLocked(time.Now()) + if len(h.registeredStates) >= oauthStateMaxEntries { + h.mu.Unlock() + http.Error(w, "OAuth login temporarily unavailable", http.StatusServiceUnavailable) + return + } h.registeredStates[state] = &oauth2State{ providerConfig: provider, providerName: providerName, Username: "", + createdAt: time.Now(), } h.mu.Unlock() @@ -177,6 +203,9 @@ func (h *OAuth2Handler) checkOAuthCallbackCookie(w http.ResponseWriter, r *http. if !h.validateStateMatch(r.URL.Query().Get("state"), state) { log.Errorf("State mismatch: %v != %v", r.URL.Query().Get("state"), state) + h.mu.Lock() + h.deleteOAuthStateLocked(state) + h.mu.Unlock() http.Error(w, "State mismatch", http.StatusBadRequest) return nil, state, false } @@ -186,6 +215,9 @@ func (h *OAuth2Handler) checkOAuthCallbackCookie(w http.ResponseWriter, r *http. h.mu.RUnlock() if !ok { log.Errorf("State not found in server: %v", state) + h.mu.Lock() + h.deleteOAuthStateLocked(state) + h.mu.Unlock() http.Error(w, "State not found in server", http.StatusBadRequest) return nil, state, false } diff --git a/service/internal/auth/otoauth2/restapi_auth_oauth2_test.go b/service/internal/auth/otoauth2/restapi_auth_oauth2_test.go new file mode 100644 index 0000000..2b6ef4f --- /dev/null +++ b/service/internal/auth/otoauth2/restapi_auth_oauth2_test.go @@ -0,0 +1,66 @@ +package otoauth2 + +import ( + "net/http" + "net/http/httptest" + "strconv" + "testing" + "time" + + config "github.com/OliveTin/OliveTin/internal/config" + "github.com/stretchr/testify/assert" + "golang.org/x/oauth2" +) + +func TestSweepExpiredOAuthStatesLocked(t *testing.T) { + h := &OAuth2Handler{ + registeredStates: make(map[string]*oauth2State), + } + + h.registeredStates["fresh"] = &oauth2State{ + providerName: "test", + createdAt: time.Now(), + } + h.registeredStates["stale"] = &oauth2State{ + providerName: "test", + createdAt: time.Now().Add(-2 * oauthStateMaxAge * time.Second), + } + + h.sweepExpiredOAuthStatesLocked(time.Now()) + + _, freshFound := h.registeredStates["fresh"] + _, staleFound := h.registeredStates["stale"] + assert.True(t, freshFound) + assert.False(t, staleFound) +} + +func TestHandleOAuthLoginRejectsWhenStateMapFull(t *testing.T) { + cfg := config.DefaultConfig() + cfg.AuthOAuth2Providers = map[string]*config.OAuth2Provider{ + "test": { + Name: "test", + ClientID: "id", + ClientSecret: "secret", + AuthUrl: "https://example.com/auth", + TokenUrl: "https://example.com/token", + }, + } + + h := NewOAuth2Handler(cfg) + h.registeredStates = make(map[string]*oauth2State, oauthStateMaxEntries) + for i := 0; i < oauthStateMaxEntries; i++ { + h.registeredStates[strconv.Itoa(i)] = &oauth2State{ + providerConfig: &oauth2.Config{}, + providerName: "test", + createdAt: time.Now(), + } + } + + req := httptest.NewRequest(http.MethodGet, "/oauth/login?provider=test", nil) + rec := httptest.NewRecorder() + + h.HandleOAuthLogin(rec, req) + + assert.Equal(t, http.StatusServiceUnavailable, rec.Code) + assert.Equal(t, oauthStateMaxEntries, len(h.registeredStates)) +}