From c24b8bda34512e4a2375644080e8061cfb71a2d1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Thu, 12 Feb 2026 19:04:30 +0100 Subject: [PATCH] fix(ui-server): prevent rate limiter memory leak Add maxTrackedIps limit with lazy cleanup to bound memory usage. Reject new IPs at capacity after cleanup (DoS protection). --- .../ui-server/AbstractUIServer.ts | 2 +- .../ui-server/UIServerSecurity.ts | 41 ++++++++++++++----- .../ui-server/UIServerSecurity.test.ts | 30 +++++++++++++- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/charging-station/ui-server/AbstractUIServer.ts b/src/charging-station/ui-server/AbstractUIServer.ts index f9d2aa0d..20bf50fc 100644 --- a/src/charging-station/ui-server/AbstractUIServer.ts +++ b/src/charging-station/ui-server/AbstractUIServer.ts @@ -22,8 +22,8 @@ import { } from '../../types/index.js' import { isEmpty, logger } from '../../utils/index.js' import { UIServiceFactory } from './ui-services/UIServiceFactory.js' -import { getUsernameAndPasswordFromAuthorizationToken } from './UIServerUtils.js' import { isValidCredential } from './UIServerSecurity.js' +import { getUsernameAndPasswordFromAuthorizationToken } from './UIServerUtils.js' const moduleName = 'AbstractUIServer' diff --git a/src/charging-station/ui-server/UIServerSecurity.ts b/src/charging-station/ui-server/UIServerSecurity.ts index 231226fe..8b6beb17 100644 --- a/src/charging-station/ui-server/UIServerSecurity.ts +++ b/src/charging-station/ui-server/UIServerSecurity.ts @@ -1,10 +1,19 @@ import { timingSafeEqual } from 'node:crypto' +/** + * Per-IP rate limiter state + */ +interface RateLimitEntry { + count: number + resetTime: number +} + export const DEFAULT_MAX_BODY_SIZE = 1048576 export const DEFAULT_RATE_LIMIT = 100 export const DEFAULT_RATE_WINDOW = 60000 export const DEFAULT_MAX_STATIONS = 100 export const DEFAULT_WS_MAX_PAYLOAD = 102400 +export const DEFAULT_MAX_TRACKED_IPS = 10000 /** * Constant-time credential comparison using crypto.timingSafeEqual @@ -46,29 +55,41 @@ export const createBodySizeLimiter = (maxBytes: number): ((chunkSize: number) => } } -/** - * Per-IP rate limiter state - */ -interface RateLimitEntry { - count: number - resetTime: number -} - /** * Creates a rate limiter function that tracks requests per IP address - * Uses a simple fixed-window approach (not sliding window) + * Uses a simple fixed-window approach with lazy cleanup to prevent memory leaks * @param maxRequests - Maximum requests allowed per window * @param windowMs - Time window in milliseconds + * @param maxTrackedIps - Maximum number of IPs to track (prevents memory exhaustion) * @returns Function that checks if IP is within rate limit */ export const createRateLimiter = ( maxRequests: number, - windowMs: number + windowMs: number, + maxTrackedIps: number = DEFAULT_MAX_TRACKED_IPS ): ((ipAddress: string) => boolean) => { const trackedIps = new Map() + const cleanupExpiredEntries = (now: number): void => { + for (const [ip, entry] of trackedIps.entries()) { + if (now >= entry.resetTime) { + trackedIps.delete(ip) + } + } + } + return (ipAddress: string): boolean => { const now = Date.now() + + // Lazy cleanup: when at capacity and new IP arrives, clean expired entries + if (trackedIps.size >= maxTrackedIps && !trackedIps.has(ipAddress)) { + cleanupExpiredEntries(now) + // If still at capacity after cleanup, reject new IPs (DoS protection) + if (trackedIps.size >= maxTrackedIps) { + return false + } + } + const entry = trackedIps.get(ipAddress) // First request from this IP or window expired diff --git a/tests/charging-station/ui-server/UIServerSecurity.test.ts b/tests/charging-station/ui-server/UIServerSecurity.test.ts index 0ee4a6ac..7a4ad9c3 100644 --- a/tests/charging-station/ui-server/UIServerSecurity.test.ts +++ b/tests/charging-station/ui-server/UIServerSecurity.test.ts @@ -79,14 +79,40 @@ await describe('UIServerSecurity test suite', async () => { limiter('10.0.0.1') expect(limiter('10.0.0.1')).toBe(false) - // Wait for window to expire await new Promise(resolve => { setTimeout(resolve, 110) }) - // Should allow request after window reset expect(limiter('10.0.0.1')).toBe(true) }) + + await it('should reject new IPs when at max tracked IPs capacity', () => { + const limiter = createRateLimiter(10, 60000, 3) + expect(limiter('192.168.1.1')).toBe(true) + expect(limiter('192.168.1.2')).toBe(true) + expect(limiter('192.168.1.3')).toBe(true) + expect(limiter('192.168.1.4')).toBe(false) + }) + + await it('should still allow existing IPs when at capacity', () => { + const limiter = createRateLimiter(10, 60000, 2) + expect(limiter('192.168.1.1')).toBe(true) + expect(limiter('192.168.1.2')).toBe(true) + expect(limiter('192.168.1.1')).toBe(true) + expect(limiter('192.168.1.2')).toBe(true) + }) + + await it('should cleanup expired entries when at capacity', async () => { + const limiter = createRateLimiter(10, 50, 2) + expect(limiter('192.168.1.1')).toBe(true) + expect(limiter('192.168.1.2')).toBe(true) + + await new Promise(resolve => { + setTimeout(resolve, 60) + }) + + expect(limiter('192.168.1.3')).toBe(true) + }) }) await describe('isValidNumberOfStations()', async () => { -- 2.43.0