diff --git a/internal/web/controller/login_limiter.go b/internal/web/controller/login_limiter.go index 1694db99e..c25d2404c 100644 --- a/internal/web/controller/login_limiter.go +++ b/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)) } diff --git a/internal/web/controller/login_limiter_test.go b/internal/web/controller/login_limiter_test.go index c6f4ac6e4..1436241f0 100644 --- a/internal/web/controller/login_limiter_test.go +++ b/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)