From 3b555a560534ca3114515c4b32737ba51b10392c Mon Sep 17 00:00:00 2001
From: Egor Seredin <4819888+agmt@users.noreply.github.com>
Date: Sat, 9 Oct 2021 09:38:51 +0900
Subject: [PATCH] ClientIP: check every proxy for trustiness (#2844)

---
 context.go      | 39 ++++++++++++++++++++-------------------
 context_test.go |  2 +-
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/context.go b/context.go
index 0ac5f8f9cc..dea08eb45a 100644
--- a/context.go
+++ b/context.go
@@ -759,7 +759,7 @@ func (c *Context) ClientIP() string {
 
 	if trusted && c.engine.ForwardedByClientIP && c.engine.RemoteIPHeaders != nil {
 		for _, headerName := range c.engine.RemoteIPHeaders {
-			ip, valid := validateHeader(c.requestHeader(headerName))
+			ip, valid := c.engine.validateHeader(c.requestHeader(headerName))
 			if valid {
 				return ip
 			}
@@ -768,6 +768,17 @@ func (c *Context) ClientIP() string {
 	return remoteIP.String()
 }
 
+func (e *Engine) isTrustedProxy(ip net.IP) bool {
+	if e.trustedCIDRs != nil {
+		for _, cidr := range e.trustedCIDRs {
+			if cidr.Contains(ip) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // RemoteIP parses the IP from Request.RemoteAddr, normalizes and returns the IP (without the port).
 // It also checks if the remoteIP is a trusted proxy or not.
 // In order to perform this validation, it will see if the IP is contained within at least one of the CIDR blocks
@@ -782,35 +793,25 @@ func (c *Context) RemoteIP() (net.IP, bool) {
 		return nil, false
 	}
 
-	if c.engine.trustedCIDRs != nil {
-		for _, cidr := range c.engine.trustedCIDRs {
-			if cidr.Contains(remoteIP) {
-				return remoteIP, true
-			}
-		}
-	}
-
-	return remoteIP, false
+	return remoteIP, c.engine.isTrustedProxy(remoteIP)
 }
 
-func validateHeader(header string) (clientIP string, valid bool) {
+func (e *Engine) validateHeader(header string) (clientIP string, valid bool) {
 	if header == "" {
 		return "", false
 	}
 	items := strings.Split(header, ",")
-	for i, ipStr := range items {
-		ipStr = strings.TrimSpace(ipStr)
+	for i := len(items) - 1; i >= 0; i-- {
+		ipStr := strings.TrimSpace(items[i])
 		ip := net.ParseIP(ipStr)
 		if ip == nil {
 			return "", false
 		}
 
-		// We need to return the first IP in the list, but,
-		// we should not early return since we need to validate that
-		// the rest of the header is syntactically valid
-		if i == 0 {
-			clientIP = ipStr
-			valid = true
+		// X-Forwarded-For is appended by proxy
+		// Check IPs in reverse order and stop when find untrusted proxy
+		if (i == 0) || (!e.isTrustedProxy(ip)) {
+			return ipStr, true
 		}
 	}
 	return
diff --git a/context_test.go b/context_test.go
index 86a29bd4a2..d1b1237480 100644
--- a/context_test.go
+++ b/context_test.go
@@ -1421,7 +1421,7 @@ func TestContextClientIP(t *testing.T) {
 
 	// Only trust RemoteAddr
 	_ = c.engine.SetTrustedProxies([]string{"40.40.40.40"})
-	assert.Equal(t, "20.20.20.20", c.ClientIP())
+	assert.Equal(t, "30.30.30.30", c.ClientIP())
 
 	// All steps are trusted
 	_ = c.engine.SetTrustedProxies([]string{"40.40.40.40", "30.30.30.30", "20.20.20.20"})