Quellcode durchsuchen

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 <cursoragent@cursor.com>
jamesread vor 7 Stunden
Ursprung
Commit
160cc5d70b

+ 32 - 0
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
 	}

+ 66 - 0
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))
+}