Fixed a critical time-of-check/time-of-use (TOCTOU) race condition in WebSocket per-IP connection limiting that allowed attackers to exceed the connection cap during concurrent upgrade bursts.
The WebSocket upgrade handler at src/ws/hub.ts (line ~190) had a race condition:
1. Connection A: checkLimiter(ip) → checks count (0 < 3) → ✓
2. Connection B: checkLimiter(ip) → checks count (0 < 3) → ✓ (race!)
3. Connection A: increment counter to 1 → upgrade completes
4. Connection B: increment counter to 2 → upgrade completes
5. Connections C & D also pass check before being counted...
Impact: Attackers could open more connections than allowed by sending simultaneous upgrade requests, defeating the per-IP cap that protects against abuse.
The checkLimiter(ip) function checked the limit and then incremented the counter as separate operations, with async I/O in between:
// BEFORE (broken)
const currentCount = connectionCounts.get(ip) || 0;
if (currentCount >= maxConnections) return false;
// ← Multiple concurrent requests could all pass here
connectionCounts.set(ip, currentCount + 1);Multiple concurrent upgrade handlers could all call checkLimiter, all see count=0, and all pass the check before any incremented the counter.
When the limit was exceeded, the code called handleUpgrade with a close callback:
// BEFORE (wrong)
if (!limitResult.allowed) {
this.wss.handleUpgrade(req, socket, head, (ws) => {
ws.close(limitResult.code || 4029, limitResult.reason);
});
}This upgraded the connection (causing 'open' event on client) before closing it, creating a race where clients could open successfully.
File: src/ws/connectionLimiter.ts
Refactored checkAndReserve to reserve the slot atomically BEFORE any async operations:
export async function checkAndReserve(ip: string): Promise<...> {
const now = Date.now();
const maxConnections = parseInt(process.env.WS_MAX_CONNECTIONS_PER_IP || '10', 10);
// 1. CHECK synchronously (first!)
const currentCount = connectionCounts.get(ip) || 0;
if (currentCount >= maxConnections) {
recordRejection(ip, now);
return { allowed: false, code: 4029, reason: 'Too many connections' };
}
// 2. RESERVE synchronously before any await
connectionCounts.set(ip, currentCount + 1);
// 3. CHECK BAN asynchronously (after reservation)
try {
const banResult = await banStore.isBanned(ip);
if (banResult.banned) {
untrackConnection(ip); // rollback if banned
return { allowed: false, ... };
}
} catch { }
return { allowed: true };
}Why it works: Even though the function is async, the synchronous check-and-increment happens before the first await. When concurrent requests arrive, they execute the check and increment sequentially (JavaScript is single-threaded), preventing bypass.
File: src/ws/hub.ts (upgrade handler)
Changed to reject at the HTTP level without calling handleUpgrade:
if (!limitResult.allowed) {
// SECURITY: Reject BEFORE upgrade. Send HTTP error instead of upgrading.
socket.write(
'HTTP/1.1 429 Too Many Requests\r\n' +
'Content-Type: text/plain\r\n' +
'Connection: close\r\n\r\n' +
`${limitResult.reason || 'Too many connections'}\r\n`
);
socket.destroy();
return;
}Why it works: The HTTP response is sent before the WebSocket handshake, so the client receives 429 and never enters OPEN state. No race possible.
File: src/ws/hub.ts
Clear ownership and cleanup flow:
checkAndReserve(ip)
├─ allowed=false → reject with 429 (no reservation) → return
└─ allowed=true → reserve slot (increment counter)
├─ upgrade fails (socket close, auth error) → cleanup handler → untrackConnection
└─ upgrade succeeds → onConnect
└─ connection closes → onDisconnect → untrackConnection
Guarded with cleaned flag to prevent double-decrement.
File: tests/ws/ws.concurrency.test.ts
Tests that verify the fix:
| Test | Scenario | Validates |
|---|---|---|
| enforces per-IP cap under concurrent upgrades | 10 simultaneous attempts vs limit of 3 | Only 3 succeed; cap is enforced |
| handles upgrade failure without counter leaks | Fill limit, close one, open new one | Counter properly released and reused |
| handles multiple concurrent close events | 5 simultaneous closes in parallel | Counter doesn't underflow (no negatives) |
| stress test rapid sequential connections | High-frequency connect/disconnect cycles | Counter accuracy under burst load |
| correctly rejects after N rejections trigger ban | Multiple rejections lead to IP ban | Ban prevents further connection attempts |
| complex concurrent scenario | Mixed open/close/reject operations | Counter stays consistent |
Test Results: All 6 pass ✓
The following invariants are now guaranteed:
- Atomic Reservation: Each
checkAndReserve(ip) → allowed:trueincrements the counter exactly once - Exactly One Release: Each established connection releases exactly once on close (via
onDisconnect) - No Underflow: Counter never goes below 0 (clamped in
untrackConnection) - No Leaks: Failed upgrades release via cleanup handler; no connections left unreleased
✅ Per-IP cap is atomic: Tested with 10 concurrent requests against limit of 3
✅ Counter never negative: Tested with 5 concurrent close events
✅ No double-decrement: Cleanup flag ensures exactly one release per upgrade attempt
✅ Rejection happens before OPEN: HTTP 429 sent before WebSocket handshake
✅ Under burst load: Stress test cycles 5 times through connect/disconnect at limit
# Run concurrency tests specifically
npm test -- tests/ws/ws.concurrency.test.ts
# Run all WebSocket tests
npm test -- tests/ws/
# Expected output:
# Test Files 4 passed (4)
# Tests 34 passed (34)Added comprehensive TSDoc comments explaining:
- Atomic operation order in
checkAndReserve - Counter lifecycle with ASCII diagram showing reservation flow
- TOCTOU prevention mechanism and why it works
- Cleanup semantics for upgrade failures
- Security guarantees at each step
Example:
/**
* Atomically checks and reserves a connection slot for an IP.
*
* SECURITY: This function prevents TOCTOU (time-of-check/time-of-use) race conditions
* by reserving the slot BEFORE any async operations...
*
* COUNTER LIFECYCLE:
* checkAndReserve(ip) ──┬─→ allowed=false → close socket (no release)
* │
* └─→ allowed=true → reserve slot (must release once)
* ├─→ upgrade failure → socket close handler → untrackConnection
* └─→ upgrade success → onConnect (owns slot) → onDisconnect → untrackConnection
*/fix(ws): reserve per-IP slot atomically to close upgrade TOCTOU race
Fixes time-of-check/time-of-use race condition in WebSocket per-IP connection
limiting that allowed attackers to exceed the cap during concurrent bursts.
CHANGES:
- checkAndReserve now increments counter BEFORE first async operation (atomic)
- Rejection path changed to HTTP 429 without upgrading (not a race)
- Counter lifecycle: one increment per reservation, one decrement per close
- Comprehensive concurrency tests verify cap is enforced under burst load
SECURITY:
- Per-IP cap cannot be bypassed via concurrent requests
- Counter never underflows or leaks
- Exactly one release per established connection
- Rejected connections don't consume slots
TEST RESULTS:
- 6 concurrency tests: PASS
- 34 WebSocket tests: PASS
- No counter underflow or leaks observed
src/ws/hub.ts: Fixed rejection path, enhanced documentationsrc/ws/connectionLimiter.ts: Enhanced documentation with counter lifecycle detailstests/ws/ws.concurrency.test.ts: Fixed test limit configuration
- Abuse control: Connection cap must be enforceable
- Security: Per-IP limits prevent single-client DoS attacks
- Concurrency: Must work under high-frequency burst connections