Merge pull request #452 from simonk52/drop-untrusted-proxy-values

Drop untrusted values from trusted proxy headers

CONTRIBUTORS.txt

@@ -148,3 +148,5 @@ Contributors
 - Frank Krick, 2018-10-29
 
 - Jonathan Vanasco, 2022-11-15
+
+- Simon King, 2024-11-12

src/waitress/proxy_headers.py

@@ -84,7 +84,8 @@ def raise_for_multiple_values():
         try:
             forwarded_for = []
 
-            for forward_hop in environ["HTTP_X_FORWARDED_FOR"].split(","):
+            raw_forwarded_for = environ["HTTP_X_FORWARDED_FOR"].split(",")
+            for forward_hop in raw_forwarded_for:
                 forward_hop = forward_hop.strip()
                 forward_hop = undquote(forward_hop)
 
@@ -103,6 +104,9 @@ def raise_for_multiple_values():
             client_addr = forwarded_for[0]
 
             untrusted_headers.remove("X_FORWARDED_FOR")
+            environ["HTTP_X_FORWARDED_FOR"] = ",".join(
+                raw_forwarded_for[-trusted_proxy_count:]
+            ).strip()
         except Exception as ex:
             raise MalformedProxyHeader(
                 "X-Forwarded-For", str(ex), environ["HTTP_X_FORWARDED_FOR"]
@@ -115,7 +119,8 @@ def raise_for_multiple_values():
         try:
             forwarded_host_multiple = []
 
-            for forward_host in environ["HTTP_X_FORWARDED_HOST"].split(","):
+            raw_forwarded_host = environ["HTTP_X_FORWARDED_HOST"].split(",")
+            for forward_host in raw_forwarded_host:
                 forward_host = forward_host.strip()
                 forward_host = undquote(forward_host)
                 forwarded_host_multiple.append(forward_host)
@@ -124,6 +129,9 @@ def raise_for_multiple_values():
             forwarded_host = forwarded_host_multiple[0]
 
             untrusted_headers.remove("X_FORWARDED_HOST")
+            environ["HTTP_X_FORWARDED_HOST"] = ",".join(
+                raw_forwarded_host[-trusted_proxy_count:]
+            ).strip()
         except Exception as ex:
             raise MalformedProxyHeader(
                 "X-Forwarded-Host", str(ex), environ["HTTP_X_FORWARDED_HOST"]
@@ -163,8 +171,9 @@ def raise_for_multiple_values():
     # If the Forwarded header exists, it gets priority
     if forwarded:
         proxies = []
+        raw_forwarded = forwarded.split(",")
         try:
-            for forwarded_element in forwarded.split(","):
+            for forwarded_element in raw_forwarded:
                 # Remove whitespace that may have been introduced when
                 # appending a new entry
                 forwarded_element = forwarded_element.strip()
@@ -213,6 +222,9 @@ def raise_for_multiple_values():
             raise MalformedProxyHeader("Forwarded", str(ex), environ["HTTP_FORWARDED"])
 
         proxies = proxies[-trusted_proxy_count:]
+        environ["HTTP_FORWARDED"] = ",".join(
+            raw_forwarded[-trusted_proxy_count:]
+        ).strip()
 
         # Iterate backwards and fill in some values, the oldest entry that
         # contains the information we expect is the one we use. We expect

tests/test_proxy_headers.py

@@ -225,6 +225,7 @@ def test_parse_proxy_headers_forwared_for_multiple(self):
 
         environ = inner.environ
         self.assertEqual(environ["REMOTE_ADDR"], "198.51.100.2")
+        self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")
 
     def test_parse_forwarded_multiple_proxies_trust_only_two(self):
         inner = DummyApp()
@@ -251,6 +252,10 @@ def test_parse_forwarded_multiple_proxies_trust_only_two(self):
         self.assertEqual(environ["SERVER_NAME"], "example.com")
         self.assertEqual(environ["HTTP_HOST"], "example.com:8080")
         self.assertEqual(environ["SERVER_PORT"], "8080")
+        self.assertEqual(
+            environ["HTTP_FORWARDED"],
+            "For=198.51.100.2;host=example.com:8080, For=203.0.113.1",
+        )
 
     def test_parse_forwarded_multiple_proxies(self):
         inner = DummyApp()
@@ -278,6 +283,11 @@ def test_parse_forwarded_multiple_proxies(self):
         self.assertEqual(environ["HTTP_HOST"], "example.com:8443")
         self.assertEqual(environ["SERVER_PORT"], "8443")
         self.assertEqual(environ["wsgi.url_scheme"], "https")
+        self.assertEqual(
+            environ["HTTP_FORWARDED"],
+            'for="[2001:db8::1]:3821";host="example.com:8443";proto="https", '
+            'for=192.0.2.1;host="example.internal:8080"',
+        )
 
     def test_parse_forwarded_multiple_proxies_minimal(self):
         inner = DummyApp()
@@ -304,6 +314,10 @@ def test_parse_forwarded_multiple_proxies_minimal(self):
         self.assertEqual(environ["HTTP_HOST"], "example.org")
         self.assertEqual(environ["SERVER_PORT"], "443")
         self.assertEqual(environ["wsgi.url_scheme"], "https")
+        self.assertEqual(
+            environ["HTTP_FORWARDED"],
+            'for="[2001:db8::1]";proto="https", for=192.0.2.1;host="example.org"',
+        )
 
     def test_parse_proxy_headers_forwarded_host_with_port(self):
         inner = DummyApp()
@@ -332,6 +346,7 @@ def test_parse_proxy_headers_forwarded_host_with_port(self):
         self.assertEqual(environ["SERVER_NAME"], "example.com")
         self.assertEqual(environ["HTTP_HOST"], "example.com:8080")
         self.assertEqual(environ["SERVER_PORT"], "8080")
+        self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")
 
     def test_parse_proxy_headers_forwarded_host_without_port(self):
         inner = DummyApp()
@@ -360,6 +375,7 @@ def test_parse_proxy_headers_forwarded_host_without_port(self):
         self.assertEqual(environ["SERVER_NAME"], "example.com")
         self.assertEqual(environ["HTTP_HOST"], "example.com")
         self.assertEqual(environ["SERVER_PORT"], "80")
+        self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")
 
     def test_parse_proxy_headers_forwarded_host_with_forwarded_port(self):
         inner = DummyApp()
@@ -390,6 +406,7 @@ def test_parse_proxy_headers_forwarded_host_with_forwarded_port(self):
         self.assertEqual(environ["SERVER_NAME"], "example.com")
         self.assertEqual(environ["HTTP_HOST"], "example.com:8080")
         self.assertEqual(environ["SERVER_PORT"], "8080")
+        self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")
 
     def test_parse_proxy_headers_forwarded_host_multiple_with_forwarded_port(self):
         inner = DummyApp()
@@ -420,6 +437,8 @@ def test_parse_proxy_headers_forwarded_host_multiple_with_forwarded_port(self):
         self.assertEqual(environ["SERVER_NAME"], "example.com")
         self.assertEqual(environ["HTTP_HOST"], "example.com:8080")
         self.assertEqual(environ["SERVER_PORT"], "8080")
+        self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "198.51.100.2, 203.0.113.1")
+        self.assertEqual(environ["HTTP_X_FORWARDED_HOST"], "example.com, example.org")
 
     def test_parse_proxy_headers_forwarded_host_multiple_with_forwarded_port_limit_one_trusted(
         self,
@@ -452,6 +471,8 @@ def test_parse_proxy_headers_forwarded_host_multiple_with_forwarded_port_limit_o
         self.assertEqual(environ["SERVER_NAME"], "example.org")
         self.assertEqual(environ["HTTP_HOST"], "example.org:8080")
         self.assertEqual(environ["SERVER_PORT"], "8080")
+        self.assertEqual(environ["HTTP_X_FORWARDED_FOR"], "203.0.113.1")
+        self.assertEqual(environ["HTTP_X_FORWARDED_HOST"], "example.org")
 
     def test_parse_forwarded(self):
         inner = DummyApp()