-
Notifications
You must be signed in to change notification settings - Fork 34
Open
Labels
Description
Problem
Critical Bug: The current rate limiter (gateway/ratelimit.go) uses the request Nonce as the key for authenticated requests.
Since the client MUST generate a unique Nonce for every request (to prevent replay attacks), every request gets a new rate limit bucket.
This results in:
- Infinite rate limits for authenticated users.
- Memory leaks as unique buckets are created for every request until
cleanupTTL.
sequenceDiagram
participant A as Attacker
participant G as Gateway
participant RL as RateLimiter
Note over A,G: Current Vulnerable Logic
A->>G: Request 1 (Nonce A)
G->>RL: Check "nonce:A"
RL-->>G: Allow (New Bucket)
A->>G: Request 2 (Nonce B)
G->>RL: Check "nonce:B"
RL-->>G: Allow (New Bucket)
Note over A,G: Proposed Secure Logic
A->>G: Request 1 (IP 1.2.3.4)
G->>RL: Check "ip:1.2.3.4"
RL-->>G: Allow (Bucket hits: 1)
A->>G: Request 100 (IP 1.2.3.4)
G->>RL: Check "ip:1.2.3.4"
RL-->>G: Deny (429 Too Many Requests)
Solution
Change the rate limiting strategy to key off IP Address even for authenticated requests, or implement a "Tentative" rate limit based on a claimed X-Wallet-Address header. Given the architecture, IP-based limiting is the safest immediate fix.
Implementation
Update getRateLimitKey in gateway/ratelimit.go:
func getRateLimitKey(c *gin.Context) string {
// REMOVED: Nonce-based keying
// ALWAYS use IP for now to prevent infinite-bucket attacks
return "ip:" + c.ClientIP()
}Acceptance Criteria
- Rate limiter correctly throttles a single user sending unique nonces
-
TestRateLimiterunit test updated to remove nonce-based keying logic - Memory usage remains stable under load
Testing
# Run a loop sending 100 requests with different nonces
# Verify that after N requests (tier limit), the server returns 429Reactions are currently unavailable