From cf48a323a60f3c9870de3ce0d5cc10f24d983ca7 Mon Sep 17 00:00:00 2001
From: Shreekrishna Chattaraj <shreekrishna1930@gmail.com>
Date: Tue, 21 Jul 2026 00:49:29 +0530
Subject: [PATCH] Security: gate X-Forwarded-For behind trusted proxies with
 strict IP validation

---
 backend/secuscan/rate_limiter.py     | 28 ++++++++++----
 testing/backend/test_rate_limiter.py | 57 +++++++++++++++++++++++++++-
 2 files changed, 76 insertions(+), 9 deletions(-)

diff --git a/backend/secuscan/rate_limiter.py b/backend/secuscan/rate_limiter.py
index 6809d08ab..7a52321f4 100644
--- a/backend/secuscan/rate_limiter.py
+++ b/backend/secuscan/rate_limiter.py
@@ -21,6 +21,7 @@
 """
 
 import logging
+import ipaddress
 import time
 from collections import defaultdict
 from typing import Dict, List, Optional
@@ -28,6 +29,8 @@
 import redis.asyncio as aioredis
 from fastapi import HTTPException, Request, status
 
+from .config import settings
+
 logger = logging.getLogger(__name__)
 
 
@@ -70,14 +73,25 @@ async def reset(self) -> None:
     def _get_client_ip(self, request: Request) -> str:
         """
         Extract the real client IP.
-        Checks X-Forwarded-For first (for reverse-proxy / Docker deployments),
-        falls back to direct connection address.
+        Only trusts X-Forwarded-For when the direct client is a trusted proxy,
+        then falls back to the direct connection address.
         """
-        forwarded_for = request.headers.get("X-Forwarded-For")
-        if forwarded_for:
-            # X-Forwarded-For can be a comma-separated list; take the first
-            return forwarded_for.split(",")[0].strip()
-        return request.client.host if request.client else "unknown"
+        client_ip = request.client.host if request.client else None
+        trusted_proxies = settings.trusted_proxies or []
+
+        if client_ip and client_ip in trusted_proxies:
+            forwarded_for = request.headers.get("X-Forwarded-For")
+            if forwarded_for:
+                # X-Forwarded-For can be a comma-separated list; take the first
+                forwarded_ip = forwarded_for.split(",", 1)[0].strip()
+                if forwarded_ip:
+                    try:
+                        ipaddress.ip_address(forwarded_ip)
+                        return forwarded_ip
+                    except ValueError:
+                        pass
+
+        return client_ip or "unknown_client"
 
     def _make_key(self, ip: str, window_type: str, window_value: int) -> str:
         """Build a namespaced Redis key for this IP and time window."""
diff --git a/testing/backend/test_rate_limiter.py b/testing/backend/test_rate_limiter.py
index a3a25a3de..67c13c4c3 100644
--- a/testing/backend/test_rate_limiter.py
+++ b/testing/backend/test_rate_limiter.py
@@ -218,13 +218,66 @@ async def test_uses_first_ip_from_forwarded_for_header(self):
             burst_limit=10,
             burst_window=3600,
         )
-        # Simulate multi-hop X-Forwarded-For
+        # Simulate multi-hop X-Forwarded-For from a trusted proxy
         request = _make_mock_request_forwarded("203.0.113.5, 10.0.0.1, 172.16.0.1")
-        await limiter.check(request)
+
+        with patch("backend.secuscan.rate_limiter.settings.trusted_proxies", ["127.0.0.1", "10.0.0.1"]):
+            await limiter.check(request)
 
         incr_calls = str(pipe.incr.call_args_list)
         assert "203.0.113.5" in incr_calls
 
+    @pytest.mark.asyncio
+    async def test_ignores_malformed_forwarded_for_and_uses_proxy_ip_fallback(self):
+        mock_redis = AsyncMock()
+        pipe = AsyncMock()
+        pipe.execute = AsyncMock(return_value=[1, True])
+        mock_redis.pipeline = MagicMock(return_value=pipe)
+
+        limiter = ScanRateLimiter(
+            redis_client=mock_redis,
+            rate_limit=5,
+            rate_window=60,
+            burst_limit=10,
+            burst_window=3600,
+        )
+
+        request = _make_mock_request(ip="10.0.0.1")
+        request.client.host = "10.0.0.1"
+        request.headers = {"X-Forwarded-For": "not-an-ip"}
+
+        with patch("backend.secuscan.rate_limiter.settings.trusted_proxies", ["10.0.0.1"]):
+            await limiter.check(request)
+
+        incr_calls = str(pipe.incr.call_args_list)
+        assert "10.0.0.1" in incr_calls
+        assert "not-an-ip" not in incr_calls
+
+    @pytest.mark.asyncio
+    async def test_ignores_forwarded_for_from_untrusted_proxy(self):
+        mock_redis = AsyncMock()
+        pipe = AsyncMock()
+        pipe.execute = AsyncMock(return_value=[1, True])
+        mock_redis.pipeline = MagicMock(return_value=pipe)
+
+        limiter = ScanRateLimiter(
+            redis_client=mock_redis,
+            rate_limit=5,
+            rate_window=60,
+            burst_limit=10,
+            burst_window=3600,
+        )
+        request = _make_mock_request(ip="198.51.100.2")
+        request.client.host = "198.51.100.2"
+        request.headers = {"X-Forwarded-For": "203.0.113.5, 10.0.0.1"}
+
+        with patch("backend.secuscan.rate_limiter.settings.trusted_proxies", ["127.0.0.1", "10.0.0.1"]):
+            await limiter.check(request)
+
+        incr_calls = str(pipe.incr.call_args_list)
+        assert "198.51.100.2" in incr_calls
+        assert "203.0.113.5" not in incr_calls
+
 
 class TestScanRateLimiterRedisError:
     """Should fail open on Redis errors."""