fix: use last IP from X-Forwarded-For to prevent rate limit bypass#1033
Open
RyoSXu wants to merge 1 commit into
Open
fix: use last IP from X-Forwarded-For to prevent rate limit bypass#1033RyoSXu wants to merge 1 commit into
RyoSXu wants to merge 1 commit into
Conversation
When the server is configured with proxied: true, addressOfRequest() was returning the first (leftmost) IP from the X-Forwarded-For header. This value is client-controlled and can be spoofed, allowing attackers to bypass the login rate limit by rotating the spoofed IP on each request. The fix changes the function to use the last (rightmost) IP instead, which is the one appended by the trusted reverse proxy. This ensures the rate limiter always sees the real client IP address. Changes: - Modified addressOfRequest() to return the last IP in X-Forwarded-For with strings.TrimSpace() to handle whitespace - Added 8 test cases covering: non-proxied mode, missing XFF header, single IP, multiple IPs, spoofed first IP, empty XFF, trailing comma fallback, and whitespace trimming Fixes glanceapp#1031
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the server is configured with
proxied: true,addressOfRequest()was returning the first (leftmost) IP from theX-Forwarded-Forheader. This value is client-controlled and can be spoofed, allowing attackers to bypass the login rate limit by rotating the spoofed IP on each request.The fix changes the function to use the last (rightmost) IP instead, which is the one appended by the trusted reverse proxy (e.g., nginx with
$proxy_add_x_forwarded_for). This ensures the rate limiter always sees the real client IP address.Changes
internal/glance/glance.go: ModifiedaddressOfRequest()to return the last IP inX-Forwarded-Forwithstrings.TrimSpace()to handle whitespaceinternal/glance/address_test.go(new): 8 test cases covering:Testing
TestAuthTokenGenerationAndVerification)go test ./internal/glance/ -vpassesFixes #1031