Преглед на файлове

fix(security): bound the login-limiter attempts map

The login rate limiter keys its records on the caller-supplied username and only
evicted a record when that exact key was revisited or the login succeeded. An
unauthenticated attacker replaying one CSRF token while rotating a fresh username
per request seeded a record that was never revisited, growing the map without
bound until the panel OOMs. Cap the map: before inserting a new record, reclaim
records whose block has lapsed and whose failures aged out, and if the map is
still at the ceiling under a broad flood, drop one so memory can never grow past
the cap.
MHSanaei преди 1 ден
родител
ревизия
0f9099149e
променени са 2 файла, в които са добавени 52 реда и са изтрити 1 реда
  1. 33 1
      internal/web/controller/login_limiter.go
  2. 19 0
      internal/web/controller/login_limiter_test.go

+ 33 - 1
internal/web/controller/login_limiter.go

@@ -10,6 +10,10 @@ const (
 	loginLimitMaxFailures = 5
 	loginLimitWindow      = 5 * time.Minute
 	loginLimitCooldown    = 15 * time.Minute
+	// Hard ceiling on tracked (ip, username) records. The key includes the
+	// caller-supplied username, so an unauthenticated attacker rotating
+	// usernames would otherwise grow the map without bound.
+	loginLimitMaxRecords = 10000
 )
 
 var defaultLoginLimiter = newLoginLimiter(loginLimitMaxFailures, loginLimitWindow, loginLimitCooldown)
@@ -63,13 +67,14 @@ func (l *loginLimiter) registerFailure(ip, username string) (time.Time, bool) {
 	l.mu.Lock()
 	defer l.mu.Unlock()
 
+	now := l.now()
 	key := loginLimitKey(ip, username)
 	record := l.attempts[key]
 	if record == nil {
+		l.evictForRoom(now)
 		record = &loginLimitRecord{}
 		l.attempts[key] = record
 	}
-	now := l.now()
 	record.failures = pruneLoginFailures(record.failures, now.Add(-l.window))
 	record.failures = append(record.failures, now)
 	if len(record.failures) >= l.maxFailures {
@@ -86,6 +91,33 @@ func (l *loginLimiter) registerSuccess(ip, username string) {
 	delete(l.attempts, loginLimitKey(ip, username))
 }
 
+// evictForRoom keeps the attempts map bounded before inserting a new record.
+// It first reclaims records that are no longer blocked and whose failures have
+// aged out of the window; if the map is still at the ceiling (a genuine
+// broad flood), it drops one arbitrary record so memory can never grow past the
+// cap. Callers hold l.mu.
+func (l *loginLimiter) evictForRoom(now time.Time) {
+	if len(l.attempts) < loginLimitMaxRecords {
+		return
+	}
+	cutoff := now.Add(-l.window)
+	for key, record := range l.attempts {
+		if now.Before(record.blockedUntil) {
+			continue
+		}
+		record.failures = pruneLoginFailures(record.failures, cutoff)
+		if len(record.failures) == 0 {
+			delete(l.attempts, key)
+		}
+	}
+	if len(l.attempts) >= loginLimitMaxRecords {
+		for key := range l.attempts {
+			delete(l.attempts, key)
+			break
+		}
+	}
+}
+
 func loginLimitKey(ip, username string) string {
 	return strings.TrimSpace(ip) + "\x00" + strings.ToLower(strings.TrimSpace(username))
 }

+ 19 - 0
internal/web/controller/login_limiter_test.go

@@ -1,10 +1,29 @@
 package controller
 
 import (
+	"strconv"
 	"testing"
 	"time"
 )
 
+// An unauthenticated attacker can flood /login with fresh usernames, each of
+// which seeds a record keyed on that username. The attempts map must stay
+// bounded rather than growing until the process OOMs.
+func TestLoginLimiterBoundsMemoryUnderUsernameFlood(t *testing.T) {
+	limiter := newLoginLimiter(5, 5*time.Minute, 15*time.Minute)
+	for i := 0; i < loginLimitMaxRecords+100; i++ {
+		limiter.registerFailure("1.2.3.4", "user-"+strconv.Itoa(i))
+	}
+
+	limiter.mu.Lock()
+	n := len(limiter.attempts)
+	limiter.mu.Unlock()
+
+	if n > loginLimitMaxRecords {
+		t.Fatalf("attempts map grew to %d, exceeding the %d ceiling under a username flood", n, loginLimitMaxRecords)
+	}
+}
+
 func TestLoginLimiterBlocksAfterConfiguredFailures(t *testing.T) {
 	now := time.Date(2026, 5, 6, 12, 0, 0, 0, time.UTC)
 	limiter := newLoginLimiter(5, 5*time.Minute, 15*time.Minute)