From 98611ca452aa82bcbe04cf140e0ba7ee8f230a4c Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Tue, 10 Mar 2026 09:10:43 +0300 Subject: [PATCH] api/handler: fix X-Real-IP header validation When trusted proxy headers are enabled, the code accepted `X-Real-IP` without validating it. The attacker could simply set `X-Real-IP` to an arbitrary and that IP would be recorded as is. We validate the IP format and ensure it's not from a trusted proxy, and add test cases. Signed-off-by: NotAShelf Change-Id: Ic1e761ea623a69371a28ad15d465d6c66a6a6964 --- internal/api/handler.go | 7 ++- internal/api/handler_test.go | 101 +++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/internal/api/handler.go b/internal/api/handler.go index 83c8e65..aa8aeba 100644 --- a/internal/api/handler.go +++ b/internal/api/handler.go @@ -272,7 +272,12 @@ func (h *IngestionHandler) extractIP(r *http.Request) string { // Check X-Real-IP header if xri := r.Header.Get("X-Real-IP"); xri != "" { - return xri + // Validate the IP format and ensure it's not from a trusted proxy + if testIP := net.ParseIP(xri); testIP != nil { + if !h.ipInNetworks(xri, h.trustedNetworks) { + return xri + } + } } // Fall back to RemoteAddr diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go index cee1dbb..0ba35d1 100644 --- a/internal/api/handler_test.go +++ b/internal/api/handler_test.go @@ -218,6 +218,107 @@ func newTestHandler(cfg *config.Config) *IngestionHandler { return NewIngestionHandler(cfg, pathNorm, pathRegistry, refRegistry, metricsAgg) } +func TestExtractIP(t *testing.T) { + cfg := &config.Config{ + Site: config.SiteConfig{ + Domains: []string{"example.com"}, + }, + Limits: config.LimitsConfig{ + MaxPaths: 100, + MaxSources: 50, + }, + Security: config.SecurityConfig{ + TrustedProxies: []string{"10.0.0.0/8", "192.168.1.1"}, + }, + } + h := newTestHandler(cfg) + + tests := []struct { + name string + remoteAddr string + headers map[string]string + want string + }{ + { + name: "direct connection no proxy", + remoteAddr: "192.168.1.100:12345", + headers: map[string]string{}, + want: "192.168.1.100", + }, + { + name: "trusted proxy with X-Forwarded-For", + remoteAddr: "10.0.0.1:12345", + headers: map[string]string{ + "X-Forwarded-For": "203.0.113.1, 10.0.0.5", + }, + want: "203.0.113.1", + }, + { + name: "trusted proxy with X-Real-IP", + remoteAddr: "10.0.0.1:12345", + headers: map[string]string{ + "X-Real-IP": "203.0.113.2", + }, + want: "203.0.113.2", + }, + { + name: "X-Real-IP from trusted network should be ignored", + remoteAddr: "10.0.0.1:12345", + headers: map[string]string{ + "X-Real-IP": "10.0.0.50", // trusted network, should fall back + }, + want: "10.0.0.1", // falls back to remoteAddr + }, + { + name: "X-Real-IP invalid IP should be ignored", + remoteAddr: "10.0.0.1:12345", + headers: map[string]string{ + "X-Real-IP": "not-an-ip", + }, + want: "10.0.0.1", // falls back + }, + { + name: "untrusted proxy X-Forwarded-For ignored", + remoteAddr: "203.0.113.50:12345", + headers: map[string]string{ + "X-Forwarded-For": "1.2.3.4", + }, + want: "203.0.113.50", // uses remoteAddr, ignores header + }, + { + name: "untrusted proxy X-Real-IP ignored", + remoteAddr: "203.0.113.50:12345", + headers: map[string]string{ + "X-Real-IP": "1.2.3.4", + }, + want: "203.0.113.50", // uses remoteAddr, ignores header + }, + { + name: "X-Forwarded-For all trusted falls back", + remoteAddr: "10.0.0.1:12345", + headers: map[string]string{ + "X-Forwarded-For": "10.0.0.2, 10.0.0.3", + }, + want: "10.0.0.1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest("POST", "/api/event", nil) + req.RemoteAddr = tt.remoteAddr + for k, v := range tt.headers { + req.Header.Set(k, v) + } + + got := h.extractIP(req) + if got != tt.want { + t.Errorf("extractIP() = %q, want %q", got, tt.want) + } + }) + } +} + func TestClassifyDevice_UA(t *testing.T) { cfg := &config.Config{ Limits: config.LimitsConfig{