Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some proxies may add port number to X-Forwarded-For header. #533

Merged
merged 1 commit into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions xrayhttp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,26 +157,50 @@ func getURL(r *http.Request) string {
return proto + "://" + r.Host + r.URL.Path
}

// clientIP returns the client IP address from the request.
//
// If the request passes through a proxy, clientIP parses
// the Forwarded header and the X-Forwarded-For header to
// find the client IP address.
// The clientIP always trusts the values of the Forwarded header
// and the X-Forwarded-For header. Please note
// that if these headers have been altered by an attacker,
// there is a possibility of returning incorrect results.
func clientIP(r *http.Request) (string, bool) {
// Parse Forwarded header.
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded
forwarded, err := forwardedheader.Parse(r.Header.Values("Forwarded"))
if err == nil && len(forwarded) > 0 {
ip := forwarded[0].For.IP
if ip.IsValid() {
return ip.String(), true
}
}

// Parse X-Forwarded-For header.
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For
forwardedFor := r.Header.Get("X-Forwarded-For")
if forwardedFor != "" {
if idx := strings.IndexByte(forwardedFor, ','); idx > 0 {
forwardedFor = forwardedFor[:idx]
}
return strings.TrimSpace(forwardedFor), true
forwardedFor = strings.TrimSpace(forwardedFor)

// some proxies may add port number to X-Forwarded-For header.
// e.g. AWS ALB: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/x-forwarded-headers.html
return getHost(forwardedFor), true
}
ip, _, err := net.SplitHostPort(r.RemoteAddr)
return getHost(r.RemoteAddr), false
}
Comment on lines +160 to +194
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clientIP function has been updated to handle cases where the X-Forwarded-For header includes port numbers. However, there is a potential issue with the getHost function call at line 191. It assumes that the X-Forwarded-For header will contain an IP address followed by a port, but this is not always the case. The getHost function should handle cases where there is no port number present.

Additionally, there is no validation to ensure that the IP address extracted is a valid format before returning it. This could lead to potential security issues if the header is manipulated.

func clientIP(r *http.Request) (string, bool) {
	// Parse Forwarded header.
	forwarded, err := forwardedheader.Parse(r.Header.Values("Forwarded"))
	if err == nil && len(forwarded) > 0 {
		ip := forwarded[0].For.IP
		if ip.IsValid() {
			return ip.String(), true
		}
	}

	// Parse X-Forwarded-For header.
	forwardedFor := r.Header.Get("X-Forwarded-For")
	if forwardedFor != "" {
		if idx := strings.IndexByte(forwardedFor, ','); idx > 0 {
			forwardedFor = forwardedFor[:idx]
		}
		forwardedFor = strings.TrimSpace(forwardedFor)

		// some proxies may add port number to X-Forwarded-For header.
		// e.g. AWS ALB: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/x-forwarded-headers.html
-		return getHost(forwardedFor), true
+		host, _ := getHost(forwardedFor)
+		if host == "" {
+			return "", false // or handle the error appropriately
+		}
+		return host, true
	}
	return getHost(r.RemoteAddr), false
}

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// clientIP returns the client IP address from the request.
//
// If the request passes through a proxy, clientIP parses
// the Forwarded header and the X-Forwarded-For header to
// find the client IP address.
// The clientIP always trusts the values of the Forwarded header
// and the X-Forwarded-For header. Please note
// that if these headers have been altered by an attacker,
// there is a possibility of returning incorrect results.
func clientIP(r *http.Request) (string, bool) {
// Parse Forwarded header.
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded
forwarded, err := forwardedheader.Parse(r.Header.Values("Forwarded"))
if err == nil && len(forwarded) > 0 {
ip := forwarded[0].For.IP
if ip.IsValid() {
return ip.String(), true
}
}
// Parse X-Forwarded-For header.
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For
forwardedFor := r.Header.Get("X-Forwarded-For")
if forwardedFor != "" {
if idx := strings.IndexByte(forwardedFor, ','); idx > 0 {
forwardedFor = forwardedFor[:idx]
}
return strings.TrimSpace(forwardedFor), true
forwardedFor = strings.TrimSpace(forwardedFor)
// some proxies may add port number to X-Forwarded-For header.
// e.g. AWS ALB: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/x-forwarded-headers.html
return getHost(forwardedFor), true
}
ip, _, err := net.SplitHostPort(r.RemoteAddr)
return getHost(r.RemoteAddr), false
}
// clientIP returns the client IP address from the request.
//
// If the request passes through a proxy, clientIP parses
// the Forwarded header and the X-Forwarded-For header to
// find the client IP address.
// The clientIP always trusts the values of the Forwarded header
// and the X-Forwarded-For header. Please note
// that if these headers have been altered by an attacker,
// there is a possibility of returning incorrect results.
func clientIP(r *http.Request) (string, bool) {
// Parse Forwarded header.
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded
forwarded, err := forwardedheader.Parse(r.Header.Values("Forwarded"))
if err == nil && len(forwarded) > 0 {
ip := forwarded[0].For.IP
if ip.IsValid() {
return ip.String(), true
}
}
// Parse X-Forwarded-For header.
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For
forwardedFor := r.Header.Get("X-Forwarded-For")
if forwardedFor != "" {
if idx := strings.IndexByte(forwardedFor, ','); idx > 0 {
forwardedFor = forwardedFor[:idx]
}
forwardedFor = strings.TrimSpace(forwardedFor)
// some proxies may add port number to X-Forwarded-For header.
// e.g. AWS ALB: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/x-forwarded-headers.html
host, _ := getHost(forwardedFor)
if host == "" {
return "", false // or handle the error appropriately
}
return host, true
}
return getHost(r.RemoteAddr), false
}


// getHost splits host and port from s.
// if s doesn't contain port, it returns s.
func getHost(s string) string {
ip, _, err := net.SplitHostPort(s)
if err != nil {
return r.RemoteAddr, false
return s
}
return ip, false
return ip
}

type rwUnwrapper interface {
Expand Down
33 changes: 33 additions & 0 deletions xrayhttp/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,39 @@ func TestClientIP(t *testing.T) {
wantIP: "198.51.100.1",
forwarded: true,
},
{
name: "xff-ipv6",
req: &http.Request{
Header: http.Header{
"X-Forwarded-For": []string{"2001:db8::1"},
},
RemoteAddr: "192.0.2.1:48011",
},
wantIP: "2001:db8::1",
forwarded: true,
},
{
name: "multiple-xff",
req: &http.Request{
Header: http.Header{
"X-Forwarded-For": []string{"198.51.100.1, 198.51.100.2"},
},
RemoteAddr: "192.0.2.1:48011",
},
wantIP: "198.51.100.1",
forwarded: true,
},
{
name: "xff-with-port",
req: &http.Request{
Header: http.Header{
"X-Forwarded-For": []string{"198.51.100.1:48013, 198.51.100.2:48012"},
},
RemoteAddr: "192.0.2.1:48011",
},
wantIP: "198.51.100.1",
forwarded: true,
},
{
name: "forwarded-header",
req: &http.Request{
Expand Down
Loading