Repository navigation
[BUG] Sliding-window limiter bypass via non-unique Redis sorted-set members #467
Description
Describe the bug
There is a rate-limiter bypass vulnerability in the SlidingWindowRateLimiter. Attackers can easily bypass the configured rate limits by sending a burst of concurrent requests in the exact same millisecond.
This happens due to two intertwined bugs:
- TOCTOU Race Condition: The
hitmethod usesPromise.allto concurrently read (getRangeFromSortedSet) and write (addToSortedSet) to Redis. When hundreds of requests arrive at the exact same millisecond, the Node.js event loop executes the "read" operation for all of them before any "write" operations finish. Redis answers0to all initial requests, allowing the entire burst to bypass the limit. - Redis Key Collision (Data Loss): The rate limiter uses the format
timestamp:step(e.g.,1775990916270:1) for Redis sorted set members. Because Redis sorted sets require unique members, when hundreds of requests arrive on the exact same millisecond, they overwrite each other. The server effectively "loses" the count of the vast majority of the burst requests.
To Reproduce
Steps to reproduce the behavior:
- In
resources/default-settings.yaml(or.nostr/settings.yaml), set a strict rate limit for testing (e.g.,message.rateLimitsto 5 per minute) and ensureipWhitelistis empty. - Start the nostream server using Docker (
./scripts/start). - Create a Node.js script using the
wspackage that opens 1000 connections and sends a dummyREQpayload simultaneously usingPromise.all(). - Run the script and count the
["NOTICE", "rate limited"]responses versus accepted responses. - See error: The server accepts significantly more requests than the configured limit (e.g., accepting ~60 requests instead of 5).
Expected behavior
The server should strictly enforce the rate limit and prevent race conditions. If the limit is 5 per minute, exactly 5 requests should be accepted, and the remaining 995 should accurately receive a "rate-limited" rejection, even if they arrive in the exact same millisecond. No data loss should occur in Redis.
System (please complete the following information):
- OS: Linux
- Platform: Docker
- Version: Latest (master)
Logs
Proof of Concept Script Output:
Connecting to ws://localhost:8008...
1000 connections established. Preparing concurrent payload...
Firing parallel requests in the exact same millisecond...
--- Test Results ---
Total Requests Sent: 1000
Accepted: 59
Rate Limited (Rejected): 941
--------------------
⚠️ BYPASS SUCCESSFUL: More requests were accepted than the configured rate limit (5) allowed.
Additional Context
Proposed Fix:
Two changes are required in sliding-window-rate-limiter.ts :
- Use a UUID to guarantee uniqueness for every hit in Redis to prevent data loss:
import { randomUUID } from 'crypto'
const member = `${now}:${step}:${randomUUID()}`
- Break apart the
Promise.allin thehitmethod andawaitthe Redis operations sequentially so the read accurately reflects the prior write :
await this.cache.addToSortedSet(key, member, now.toString())
// ...
const count = await this.cache.getRangeFromSortedSet(key, windowStart)
Activity
YashIIT0909 commentedon Apr 12, 2026
@cameri @phoenix-server should i submit a PR for this ?
saniddhyaDubey commentedon Apr 13, 2026
I believe your approach will solve the data loss, but still the race condition will exists under high concurrency. Because both the redis command, read and write, are different operations. The actual solution to this is wrapping the entire
per-requesthit function into a lua script and call lua script with the data. Lua scripts guarantees atomicity and we won't be having the overhead to maintain UUIDs and anything at all!YashIIT0909 commentedon Apr 14, 2026
I agree with your point. The UUID + sequencing fix addresses the collision/data-loss issue and significantly reduces the bypass window, but it still relies on separate Redis operations and is not fully atomic under extreme concurrency.
A Lua-script-based check-and-insert is the more correct long-term solution because it makes the decision and state update happen atomically in Redis. That would remove TOCTOU risk entirely and avoid extra member-uniqueness handling in application code.
cameri commentedon Apr 18, 2026
Assigned to @YashIIT0909 .
Please see #476 on how Lua scripts are used to avoid race conditions.